plone / plone.base

Plone Interface contracts, plus basic features and utilities
https://pypi.org/project/plone.base
2 stars 0 forks source link

Config with default template #28

Closed gforcada closed 1 year ago

gforcada commented 1 year ago

Linting and testing with GitHub Actions and pre-commit.ci

According to tox -e dependencies plone.base depends on plone.app.layout, that's the only dependency I did not declare, all others are there and fine 😄

Should we use this list, plone.base dependencies, as a list of package not to define on any other distribution that depends on plone.base and so configure z3c.dependencychecker for that? 🤔

That's the only sane way to reduce dependencies while ensuring that all needed distributions do get installed.

This is going to be more and more noticeable as we keep enabling more and more repositories to run tests with GitHub Actions.

See this very tox.ini where I had to manually install plone.portlets as it is being used in plone.protect, but not declared there 🤦🏾

CC @jensens @mauritsvanrees

mister-roboto commented 1 year ago

@gforcada thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

jensens commented 1 year ago

According to tox -e dependencies plone.base depends on plone.app.layout, that's the only dependency I did not declare, all others are there and fine smile

Good catch, like! I already prepared a two PRs to fix this.

Should we use this list, plone.base dependencies, as a list of package not to define on any other distribution that depends on plone.base and so configure z3c.dependencychecker for that? thinking

That's the only sane way to reduce dependencies while ensuring that all needed distributions do get installed.

Yes, this is a good way to start. In future we may think about respecting transitive dependencies automatically, but at the moment there is no tooling for this, right?

This is going to be more and more noticeable as we keep enabling more and more repositories to run tests with GitHub Actions. See this very tox.ini where I had to manually install plone.portlets as it is being used in plone.protect, but not declared there 🤦🏾

Indeed another good catch. IMO (OT here) plone.portlets, plone.app.portlets and all Portlet related code in CMFPlone/plone.app.layout and where ever hidden should at least in future be handled like addon code, all dependending on CMFPlone, not vice versa.

mauritsvanrees commented 1 year ago

As you correct the messages in the interfaces, which are used for display and translation, the news file should state this.

I don't think this needs special mention in a news file. This is just typo fixes. But we should alert @erral: in commit b5e318b443ce1c72d14fbec25d8c499f2f4b9119 some typos in messages are fixed. They may need some manual handling in plone.app.locales to avoid fuzzy translations.

erral commented 1 year ago

As you correct the messages in the interfaces, which are used for display and translation, the news file should state this.

I don't think this needs special mention in a news file. This is just typo fixes. But we should alert @erral: in commit b5e318b some typos in messages are fixed. They may need some manual handling in plone.app.locales to avoid fuzzy translations.

indeed. Thanks for the heads up.

mauritsvanrees commented 1 year ago

I have merged master in here. That should fix the hundreds of test failures.

@jenkins-plone-org please run jobs