Apply the cookiecutter to LMS #5444
Conversation
seanh
changed the title
seanh
force-pushed
the
apply-cookiecutter
branch
2 times, most recently
from
34c08eb to
5f22eca
Compare
|
Note to self: recompile the |
There was a problem hiding this comment.
This is too much at once to seriously review on a line by line basis, but I can test it as a black box.
As mentioned, I think we've upgraded all the dependencies by mistake, but even with this it seems to work for me. I ran through these, mostly just checking it completed and didn't spit out any errors:
make cleanmake sure(ran twice)make devmake devdatamake dockermake docker-run- Seems broken to me - Missing env settings?make sqlmake template- Resulted in quite a few changes
I didn't do any manual testing to see the app is actually working.
seanh
force-pushed
the
apply-cookiecutter
branch
2 times, most recently
from
f0677e7 to
b77f765
Compare
|
@jon-betts Could you re-review?
|
There was a problem hiding this comment.
I'm not sure if these are things you want to do anything about, but there is a conflict with the formatting of some of the requirements files and dependabot. This is due to us running pip-compile with --allow-unsafe on. If we want to resolve it it can be removed.
When running make template I still see some chnages in pyproject.toml.
| pip==23.1.2 | ||
| # via pip-tools | ||
| setuptools==67.8.0 | ||
| # via pip-tools |
There was a problem hiding this comment.
I think this is due to a difference between how we compile things and how dependabot does. If you want to resolve this so we don't get so much chatter in PRs from dependabot you can remove the --allow-unsafe from the requirements building.
There was a problem hiding this comment.
Can we see what happens with future Dependabot PRs? If it's still not using --allow-unsafe then we can remove that from the cookiecutter and apply the change to all our projects at once
| "lms/pshell.py", | ||
| "lms/migrations/*", | ||
| "lms/extensions/feature_flags/views/test.py", | ||
| "lms/views/feature_flags_test.py", |
There was a problem hiding this comment.
Running make template removes the lines from pshell.py onwards.
There was a problem hiding this comment.
This is a weird one:
make templatedoes not make any changes topshell.pyfor me even if I domake clean template- But if I clone another copy of the repo, switch to this branch, and run
make templatethen it does make the changes topshell.py😕
This is the relevant part of the cookiecutter's pyproject.toml template:
The lines that are being removed need to go in a .cookiecutter/includes/coverage/omit file. And indeed there is no such omit file on this branch, so I would expect the lines to be getting removed by make template. So why are these lines not being removed when I run make template in my original clone of this repo?
It turns out that I do have a cookiecutter/includes/coverage/omit file locally but this file is ignored by one of the cookiecutter's .gitignore rules:
I'll open an issue in the cookiecutter's repo to fix that rule. In the meantime I've force-added the missing file on this branch (even though it's .gitignore'd) so make template is clean now
There was a problem hiding this comment.
Apply our pyramid-app cookiecutter to LMS. Depends on hypothesis/cookiecutters#121
These cookiecutter PRs are by their nature kind of hard to review by looking at the diff. Some recommendations on how to test it:
ci.ymlpassing on this branchmake helpto see the available commands and try them outmake clean sureon your machine