Closed olemoign closed 2 years ago
I reckon we need to drop Python 2.7 and 3.5 at minimum to get tests to pass.
Is this something that you would be willing to do? If so, I can take care of it.
There are other things that need fixing, too.
Yes, looks like there are a few incompatibilities with pyramid 2, I can check as well.
@olemoign yes, please proceed. Thank you!
I can see that compatibility is tested with pyramid 1.3 (last release 2012), I feel we could drop 1.3 and 1.4 and start with 1.5 (last release 2016)?
I'm not clear what changes are being made that would require dropping the testing for old pyramid versions. I'm ok with it if a new feature requires it but otherwise I don't see the point.
Definitely dropping old python versions should be done.
Pyramid 1.3a1 was the first version to add Python 3 support, so it and later versions should be kept in the test bed.
Pyramid 2.0 removed the deprecated scaffolds, and pyramid_layout uses a scaffold when running tests, so they currently fail.
@mmerickel should we switch to using the cookiecutter going forward?
Would this be a compelling reason to drop older versions of Pyramid?
Yeah, I saw that. All tests are passing except for this. I don't super know how cookiecutter works, so I was planning to do the switch on Sunday. I'm re-adding the lint test that was commented. Isort is good, Flake8 in progress, I'll manage Black in the end.
Btw, the setup.py references the Repoze license at an url that doesn't exist anymore (http://www.repoze.org/LICENSE.txt). Is there a copy of it somewhere else? Is https://github.com/repoze/repoze.who/blob/master/LICENSE.txt or https://www.openhub.net/licenses/repoze-public-license good? Pyramid also references the same url in its setup.py.
Perhaps there is something in the cookiecutter tests that can be reused? Alternatively we could use its rendered files. The Pyramid docs have details about how to create a Pyramid project.
I am OK with switching to pytest, too.
For the license URL, please use https://pylonsproject.org/license.html
Looking good so far. Coverage and Lint tox envs fail at the moment.
Almost done. I've converted the scaffold to a cookiecutter. I've just spent an hour trying to understand why the translations weren't working, turns out ... the .mo files did not contain the same strings as the .po. :smile: This fixes the coverage.
Linting will always fail in cookiecutter on the lines with templating. I think we could ignore it, considering how little code there is inside. The easiest way to ignore it, would be to move the cookiecutter one level up. Touching on this, I think it would make sense to move demo and tests one level up as well. And I'm not sure what's the purpose of having a demo and a cookiecutter which are very very similar (demo is an even simpler version of the cookiecutter).
Linting will always fail in cookiecutter on the lines with templating. I think we could ignore it, considering how little code there is inside. The easiest way to ignore it, would be to move the cookiecutter one level up. Touching on this, I think it would make sense to move demo and tests one level up as well.
Indeed, we should follow the structure of Pyramid.
.
├── demo (or cookiecutter, see below)
├── docs
├── src
│ └── pyramid
└── tests
And I'm not sure what's the purpose of having a demo and a cookiecutter which are very very similar (demo is an even simpler version of the cookiecutter).
I'm confused. It looks like you made the directory cookiecutter
, which is based on pyramid-cookiecutter-starter
and incorporates the i18n feature from the original scaffold-based demo. The stuff in the directory cookiecutter
is a demo app, correct? And do you mean you don't see the point of maintaining two demos, one in demo
and another in cookiecutter
?
I'm confused. It looks like you made the directory
cookiecutter
, which is based onpyramid-cookiecutter-starter
and incorporates the i18n feature from the original scaffold-based demo. The stuff in the directorycookiecutter
is a demo app, correct? And do you mean you don't see the point of maintaining two demos, one indemo
and another incookiecutter
?
Before, there were demo
and scaffold
folders, now demo
and cookiecutter
. I modified the existing scaffold to fit with cookiecutter and yes, the end result is basically the pyramid-cookiecutter-starter
jinja2 version with added i18n. So yes, I would call that a demo app. :)
What I'm wondering is the use of having both (demo and scaffold/cookiecutter), when in my mind their use is basically the same.
I've separated cookiecutter and linting, I'll submit PRs after this one is finished.
(The only test that still fails is coverage, because the scaffold is not tested. Considering the other PRs, I consider this a pass.)
What I'm wondering is the use of having both (demo and scaffold/cookiecutter), when in my mind their use is basically the same.
I did a little archeology to try to figure this out.
https://docs.pylonsproject.org/projects/pyramid_jinja2/en/latest/changes.html
The latter converted it from a paster template to a Pyramid scaffold. I think that demo
was completely forgotten. I am OK with removing it, especially since you've brought in i18n.
The old pcreate scaffold was pretty cool in that it would set up translations and i18n, and I think it's cool that you have maintained that feature in the cookiecutter. We'll have to update the docs, once the dust settles.
https://docs.pylonsproject.org/projects/pyramid_jinja2/en/latest/index.html#pcreate-template-i18n
OK, thank you! I'll pick this up again later today and review it more carefully.
@stevepiercy @bertjwregeer, it seems the last item was to merge the github test files, I've just done it, is there anything else blocking this PR?
@olemoign I just approved the run of GHA and CI passes. Thank you!
We will need to review this PR. I'm backlogged until the weekend, but maybe @bertjwregeer or @mmerickel have a few free cycles.
@stevepiercy @bertjwregeer Gentle reminder about this MR. :smile:
@stevepiercy @bertjwregeer This MR has now been finished for more than 3 months ... :( Is there anything I can do to help you?
sorry for the delay @olemoign :-( great work on this PR, looking to release it now
I got a bit frustrated, but I 100% understand. Life takes priority over unpaid work. I have two more PRs (lint and scaffold update) that were awaiting this one, I can probably submit them on Monday. Your call if it's worth waiting.
ok I just started refactoring the source layout and adding lint so I may be taking over that part but I’m interested in what you are thinking for a scaffold update?
:smile: I have already done the lint, just moved it out of this PR because it got too big. I can't submit it until tomorrow at the earliest. Maybe it would save you some time? I'm fine with trashing it if you do it. Regarding the scaffold, I transformed it into a cookiecutter template.
I did the blacken/lint steps today but I will wait for the cookiecutter prior to release. Thank you @olemoign!
Test build fails: https://github.com/Pylons/pyramid_jinja2/pull/159/checks?check_run_id=2566704927
I reckon we need to drop Python 2.7 and 3.5 at minimum to get tests to pass.
There are other things that need fixing, too.