Unify styling and linting on plone/collective repositories

Development
, ,
1

Hi all! :wave:t4:

The ones that know me already know that I have a slight OCD on linting :joy:

Out of plone.api came a Plone styleguide that eventually was tried to be enforced by plone.recipe.codeanalysis.

From there plenty of flake8 plugins were born.

We even have our own profile for isort :sparkles:

black has rendered any formatting discussion moot (for good :smile: ).

With python, and Plone following, moving to python 3 and a year release cadence, new syntax is coming and old one is deprecated, so more tooling (pyupgrade and plenty others) have became sort of standards to ease upgrading the syntax.

Not all developers are native English speakers, and even so, we all make typos on our mother tongues :sweat_smile: So yet more tooling to help out (codespell).

All these tools are great and if you do your duty of linting a package with all of them and on top of that add some extra functionality, you are already a hero :sparkles: :superhero:t4: :cake:

Unfortunately the next contributor might miss to use these tools, so we are back to square one :man_facepalming:t4:

That's where the enforcing part comes into play.

For a while GitHub Actions are the new rage in CI, so let's use them for it :sparkles:

Configuring a single repository is not that hard, see this example PR on plone.batching, but things start getting complex when you try to lint a package that has to be still Plone 5.2 compatible, and thus maintain python 2.7 compatibility (i.e. no fancy f-strings for you :cry: ).

Again, others came up with a solution :tada: the zope folks (:wave:t4: ) came up with meta (not related to Facebook as far as I know :joy: ).

This repository has a set of scripts and configuration files that allow to mass-configure and for that unify configuration files, CI workflows and what not :star_struck:

If there is enough consensus (framework team? :thinking: should I write a PLIP?) I would volunteer to do a friendly fork of that repository to the plone (maybe collective as well) organization, and start configuring repositories with it.

As a wishlist one could start with the uncontroversial ones:

And add more tooling as we agree on, like:

At some point running the tests fo the package itself and get coverage reports would be nice, but that requires some extra effort (we can again follow the lead of the Zope organization and start using tox for it, to test on multiple python versions).

There are plenty more details that could be discussed, but this is already too long, and if there is no wish for having this it might not be needed anyway to mention them :smile:

What should be the next steps forward then?

  • build consensus here
  • write a PLIP?
  • get an early implementation
  • test the approach and submit it for feedback and approval
  • start using it on all plone packages
  • find the problematic/complex ones and improve the tool
  • keep adding more tools as we get consensus on using them

:four_leaf_clover:

5 Likes
2

It's time to unburden some of our technical debt. Thank you for leading the charge!

The Plone Documentation Team already does the following for documentation—which is not code—but you might want to consider or incorporate some of our practices. We focus on making things easy and approachable for first-time contributors.

  • Document how to contribute to your project, and keep that documentation maintained. Contributing to Documentation.
  • Use GitHub Actions Workflow Templates and Community Health Files for contributing, issues, pull requests, and more. The Plone organization provides these files by default for all projects under the organization that do not have them, and you can override them with your customizations.
  • Use make as the interface to do everything: set up a virtual environment, install requirements, build, test, deploy, ALL THE THINGS! make can wrap tox for building and testing on multiple versions of Python. make is used in Volto, too.
  • Make it easy to get started. (See what I did there? :wink:) For Plone 6 Documentation, after you have basic requirements satisfied (Python, make, Vale, and other operating system packages), you can clone the GitHub repository, and issue a single command to set up your environment, install packages, clone git submodules, and build or test the docs. Easy!
  • make linkcheckbroken checks documentation for broken or redirecting links.
  • make html will validate documentation markup.
  • make vale will use Vale to check American English spelling, grammar, and syntax, and to offer suggestions for improvement. It is highly customizable, and is useful to help authors write good better English than without it.
  • Syntax highlighting via Sphinx and Pygments. These tools validate code block sytnax and make your code more legible to the reader.
  • Use GitHub Actions to test pull requests and perform routine tasks that can be automated, such as building and deploying documentation to a production server when a pull request is merged to your project's default branch.
  • Use Netlify to preview pull requests for documentation. No more pulling down a PR to make sure it does not break docs. We have a free Open Source License for the Plone organization. Contact me if you want to use it.
  • Use MyST instead of reStructuredText or Markdown. MyST combines the simplicity of Markdown markup, with the power of reStructuredText and Sphinx.

Volto is similar to Documentation in that it has its own unique ecosystem, where quality checking tools differ from typical Python packages. The Volto team started discussion about what should be adopted and adapted to their needs. I won't go into the specific JavaScript tools used, but tools that may be useful for any project include:

  • towncrier to generate release notes from change log entries.
  • Cypress for running tests of user interfaces, including recording videos and screenshots.

I have not tried codespell, but it seems not to have as many features or customizability of Vale. We recently ditched sphinxcontrib-spelling because it was unable to recognize hyphenated words as spelled correctly. It would mark "add-on" as misspelled. Argh!

Finally, for any Python project, pyenv lets your run multiple Pythons on your system.

1 Like
3

GitHub - collective/zpretty: A tool to format in a very opinionated way HTML, XML and text containing XML snippets. for zcml formatting.

2 Likes
4

Volto is moving toward make in this issue: Move all ci scripts declared in package.json:scripts to a Makefile · Issue #3192 · plone/volto · GitHub.

Another tool to consider, pre-commit. I can't remember which Plone repo uses it, but it enforces an automatic run of linters on every commit, so garbage never makes it into PRs. It can also run a test suite, which also reduces the breaking of tests.

1 Like
5

Yeah, Makefile makes sense, to keep the same UX for both CI and local development :+1:

I'm not a huge fan of pre-commit though, I see clearly its benefits, but I'm wary of having to code and maintain yet another set of version pins and configurations.

Maybe there is a solution for that, and if we end up using a meta-project to orchestrate our config files, that might not matter much any way :smile:

Thanks for the ideas :sparkles:

1 Like
6

You may want to have a look at GitHub - release-drafter/release-drafter: Drafts your next release notes as pull requests are merged into master. and here is an example in the wild: cookiecutter/.github/workflows/drafter.yml at main · cookiecutter/cookiecutter · GitHub

7

Check translations via i18ndude would.be nice as well

8

Thanks for bringing this up! Some thoughts:

  • I would not spend time on getting this to work for repos that still support Python 2.7.
  • Some packages work only on Python 3, but still support Plone 5.2 as well. We could ignore those as well. But the linting part should be the same, except that Python 3.7 should still be supported for now.
  • Checks for i18ndude are only useful in plone.app.locales. Well, in collective packages as well.
  • I am a fan of tox for doing linting and testing. (Watch out for some incompatibilities in the recent tox version 4.)
  • I am less of a make fan, but that is mostly because I have seen too many Makefiles that are far too long, or have too many parts depending on other parts, making it very hard to understand what actually happens when you run make test or make lint, especially if there is a problem that you need to debug. In plone.dexterity there is a Makefile with just three small targets; I am happy with that. I can see the benefit of having a wrapper around several commands. But if it ends up as only a wrapper around tox, then let's simply use tox.
  • There is already a plone/code-quality Docker image, used by the plone/code-analysis-action GitHub action. plone.dexterity is using it in the Makefile linked above, and in this gh-action. Maybe this can be improved with some of your ideas.
  • Checks to include: definitely black, isort, pyupgrade, flake8. Probably zpretty. That may be enough for a first setup.
  • codespell could be interesting, or as Steve says maybe Vale, if either of those work nicely within Python code, and perhaps templates or xml as well.
  • refurb sounds very interesting, I think there is a lot of opportunity to make Plone code more Pythonic. My gut feeling with a tool like this (without having tried it at all!), is that you need to carefully consider every suggested change, instead of basically blindly accepting the fixes by tools like black and isort. So maybe not the best candidate to start with.
  • z3c.dependencychecker may also be tricky and it may suggest changes that are better done in a Plone major or minor release, especially when it tells you to drop a dependency. But with ignore-packages this may work. We will need some user mappings, especially defining which packages are included by Zope and plone.base. These could be long lists, but they can be shared in a meta package where we occasionally copy the config files from.
  • I like the Zope meta approach. I have occasionally used this to upgrade some Zope packages. Something like this is indispensable when we want to add linting and gh-actions inside individual packages and keep them up to date. Their code and config is more complex than what we need, because several Zope package have C code, which makes things a lot more complex.
  • For a first version, it might be sufficient to have a central repo with a few files that are copied. For example with a pyproject.toml, tox.ini and github/workflows/lint.yml we may come a long way for most packages, without needing further changes.
  • To make similar changes across many packages, we might use all-repos. But I have not tried this, and it may be tricky to restrict the changes to repos used by Plone 6.
  • Looking further into the future, it would be nice if we can setup gh-actions in several stages, that depend on the previous stage to pass. First do the linting. Then run the tests of the current package. Then let the final gh-action add a comment to trigger running the tests on Jenkins. This gets trickier when the PR needs a change in another package.
3 Likes
9

I want to mention the Plone Code Quality tool is already an attempt to do that. It uses pyproject.toml configuration (example config)

This tool is already the default option when you start a new project with cookiecutter-plone-starter, where you can use make format and make lint

The tool also has a GHA implementation that you could see working on collective.casestudy

It already has checks:

And it also runs as a formatter for the codebase.

I would love to add more flake-8 checks, including the usage of plone.api,

10

@gforcada, any chance you want to sprint on that during Alpine CIty Sprint?

11

Yes, by all means. I'm signed up, but unfortunately until next week I can not guarantee that I will be there physically.

If that is not possible (to be there physically) at least I will try to be remote all days but Wednesday (as that's the reason not to be 100% sure if I will be there in person).

I did have a look at the Plone Code Quality tool, though not sure if it is more for add-ons rather than core repositories? :thinking: as usual we probably can bend it to both use cases if we put enough effort :smiley:

Like @mauritsvanrees I very much see us doing a fork/copy of zopefoundation/meta repo and maintain sanity between all core repos. For linting we could use the Plone Code Quality tool if it fits the needs :+1:t4:

1 Like
12

Thanks for the long list of points, all them really good. I'm also not a huge fan of makefiles either, but as I'm also not so versed with tox.ini (almost at all :sweat_smile: ) we can see which balance we can take from it.

As for z3c.dependencychecker, indeed, is a bit tricky, but the whole point of plone.base is to untangle dependencies, so IMHO, unless one starts to specify them correctly and gets warnings when adding more undeclared dependencies, we will never get out of it :slight_smile:

I remember that there are some packages (maybe pip does that already these days) that prints the dependency graph of a given set of packages, we should add that as well, to see how the dependencies go :slight_smile:

13

The original goal of Plone Code Quality was to be used for add-ons, but the idea was to be flexible enough to support both use cases.
One of the things I wanted to have but did not have time to implement was flake8-plone-api enabled by default. For Plone 6 development, we should improve flake8-plone-api and add warnings related to plone.base.
Of course, for core repositories we could disable flake8-plone-api checks.

1 Like
14

What do you use instead?

One major reason to use make is to have a common interface across all projects, for both Python and JavaScript. In theory, make tests could run tests on any project, even if it is a wrapper around tox or the test runner. I don't know if tox works with JavaScript-based projects, but I highly doubt it would.

tox makes it easy to test a Python project across all supported versions of Python because of its configuration. I suppose one could do the same with make, but the configuration would be less elegant.

1 Like
15

Side note: I am a huge fan of Makefiles. And we are working on one way to divide the file into mungable sections with high transparency on what happens (mxmake, work in progress). I also do not like huge Makefiles (anyway, it does not scare me away). Make and its simple and excellent dependency resolution is widely adopted, very well settled, works across Languages/Systems and is well documented.

2 Likes
16

At work we are still relying on zc.buildout generating scripts on the bin folder :sweat: old but still works.

For a while, we started mixing that with virtual environments (installing things directly there bypassing zc.buildout I mean) and a few Makefile files as well (and of course the usual amount of scripts on packages.json for JS/frontend things.

But I'm starting to see the benefits of using tox for all formatting/linting/testing, specially as it gives a bit of a friendlier (once you know it) way to run commands both in CI as well as locally.

Moreover if we consider that Python is evolving on a year cadence and this means that at least once a year you have to start the dance of upgrading and deprecating python versions...

17

For the ones interested: zopefoundation/meta repository has been forked at plone organization:

And a first configuration set of files are ready to be reviewed and approved :four_leaf_clover:

A few test PRs have been created with that code:

Let me know what you think about it :smile: :sparkles: :four_leaf_clover:

18

This was announced today, and may be useful for this effort.

2 Likes
19

Indeed, it looks interesting, though maybe a bit too simple :thinking:

I mean, we can not add black as a required workflow, as that wouldbe meaningless on documentation or volto repos, same thing around with js tooling or check links verifiers...

The meta repository on zopefoundation is mostly a configuration tool for tox.ini and other configuration files, rather than specifically for GitHub workflows.

The contributor agreement would not even fit the bill, as release managers are allowed to accept on a per PR basis :thinking:

Variables on th other hand look awesome, be able to define python versions across all plone org sounds great :heart: or node version, etc

We need to explore it :+1:

20

I mean, we can not add black as a required workflow, as that wouldbe meaningless on documentation or volto repos, same thing around with js tooling or check links verifiers...

Maybe we could add required workflows, but make it possible to disable them for specific repositories in a configuration file. (i.e. the workflow would check the config file and immediately return a successful result if the check is disabled)

The contributor agreement would not even fit the bill, as release managers are allowed to accept on a per PR basis :thinking:

This is not a problem, release managers are admins and can bypass the merge requirements when there is an exception.