search

3-manifolds / SnapPy

SnapPy is a package for studying the topology and geometry of 3-manifolds, with a focus on hyperbolic structures. It is based on the SnapPea kernel written by Jeff Weeks.
https://snappy.computop.org/
84 stars 39 forks source link

trying to add a linter (codespell and pycodestyle) #85

Closed fchapoton closed 1 year ago

fchapoton commented 1 year ago

This is just a first sketch. The configuration will certainly needs to be tweaked until the lights come back green.

fchapoton commented 1 year ago

in principle, this is now ready to go. Any opinion ?

fchapoton commented 1 year ago

what do you think ? this is working now and should be very harmless.

fchapoton commented 1 year ago

now fixed and ready to go ! let's go ?

culler commented 1 year ago

@fchapoton can you say a bit about how this will work in practice? Specifically, will the CI run fail if the linter finds a stylistic error?

fchapoton commented 1 year ago

If the linter fails (usually in less than one minute), it will appear in the "Checks" windows as failed. I am not sure wether or not this will stop the rest of the process to happen. I would say no. Should we try ?

fchapoton commented 1 year ago

For example, in the similar process run in sagemath/sage, the linter failure does not stop the build and test check.

fchapoton commented 1 year ago

In case of failure, there will be errors messages clearly saying where the typo or the bad style happens (file and line number). More delicate would be the case of false typos, that would need to be added to the list of exceptions in the call to "codespell" inside .github/workflows/lint.yml.

culler commented 1 year ago

Hopefully the linter will produce the full list of all failures in the entire repo before it reports failure. Is that the case?

fchapoton commented 1 year ago

yes, it checks all the repo and reports all failures. But because it has two commands (codespell and pycodestyle), it may stop on the first one it it fails. When refreshing the branch here, I had first to fix codespell warnings, then I got the pycodestyle warnings.

fchapoton commented 1 year ago

My plan, once this is activated, is to move on: fix more pycodestyle warnings and add them to the checks. This will go through pull requests, so you will be able to refuse those that you consider controversial.

So can we please merge ?