oelint-adv icon indicating copy to clipboard operation
oelint-adv copied to clipboard

automatted reformatting / improve linting

Open HerrMuellerluedenscheid opened this issue 2 years ago • 2 comments

The linter is run as part of each module tested in the same stage. I think there are more projects that run the linter as a stage before the actual test stage to scope the problems a little better, which makes sense to me. @priv-kweihmann What do you think about running the style checks first before executing tests? Also, the tests take quite a long time to finish. It's a little annoying when those 20 minute test sessions fail because of a style issue which could have been caught at an earlier stage.

Many linters (such as black) allow automatted reformatting so that I as the contributor don't have to think about style at all. Is there a way to fix the contributed code automatically given the linters used in oelint or is there no way to get around manual labor?

HerrMuellerluedenscheid avatar Nov 03 '21 09:11 HerrMuellerluedenscheid

Well yes we could do that. I set it up like this out of the following reasons

  • one common interface (pytest)
  • one common configuration file (setup.cfg)
  • same behavior locally and in CI

I'm definitely not the pytest expert in here, so I might need a little help to untangle this into two separate stages, while keeping the above mentioned points.

Still I'm open to your idea - expect one point (:smile: )... I don't want any auto-formatting done as part of the CI. Every change done to this repo has to be in the same state as the author committed it to. Otherwise we need to have

  • a bot with write access to this repository
  • a set styleguide ( :wink: )
  • a defined way how to deal with unwanted modifications by the bot (and yeah I've been there unfortunately)

A possible way out is to promote the usage of pre-commit hooks or similar, so the modification still happens before the actual commit (that would be okay for me).

priv-kweihmann avatar Nov 03 '21 10:11 priv-kweihmann

Love your suggestion using pre-commit ! Let's do that

Just to avoid confusion: I'm not advocating to do the fixing automagically in the CI, but the verification that a user has done the linting (a la black --check). With black we don't need have a styleguide in place. That's the most charming aspect about black - It gets rid of discussion (:

To clarify: I (and probably other future contributors) would love to have something like black, make format, pre-commit, prepare-commit-without-using-my-brain.sh or alike that I can run that takes care of the style, so I don't have to manually replace double quotes with single quotes, add commas at end of lines, etc.. waste of time.

I'll give a shot overhauling the tests maybe this weekend if I find the time to rule out some pitfals.

HerrMuellerluedenscheid avatar Nov 03 '21 10:11 HerrMuellerluedenscheid