Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make repos flake8 clean and enable it on CI #643

Closed
45 tasks done
Flamefire opened this issue Aug 18, 2020 · 2 comments
Closed
45 tasks done

Make repos flake8 clean and enable it on CI #643

Flamefire opened this issue Aug 18, 2020 · 2 comments
Milestone

Comments

@Flamefire
Copy link
Contributor

Flamefire commented Aug 18, 2020

As discussed with @boegel it is a good idea to run flake8 on the whole code as part of CI to catch issues that hound misses, e.g. formatting making adjacent lines "invalid".

No not have 1 huge, unmanageable PR I propose to tackle 1 folder at a time. This has the benefit of being able to parallelize efforts. Example PR: easybuilders/easybuild-framework#3282

To get this done fast I propose to do that in a sprint with a couple people "cleaning" folders one by one. And finally enable flake8 as a Github Action on the repo root, see https://github.com/easybuilders/easybuild-framework/pull/3282/files#diff-fa7ae6d9a59a8cf2353dbb6eb115ff68

Preparation:

  • Checkout develop branch and pull if required
  • Upgrade flake8 and autopep8 (add --user if wanted or not using pyenv/virtualenv):
    • pip install --upgrade pip
    • pip install --upgrade flake8 autopep8
  • Bonus: Install VSCode, the Python Plugin and enable autopep8 as the linter

Plan to do for each folder:

  • autopep8 -r -i --max-line-length 120 <folder> to fix "simple" issues unlikely to cause problems
  • Commit
  • autopep8 -a -r -i --max-line-length 120 <folder> to fix harder issues which need to be checked at least
  • Commit
  • Iteratively run flake8 <folder> and fix issues
  • Commit
  • Open PR

On the start of the sprint all folders will be listed here or in the channel and each assigned to 1 developer. It might make sense to have 1 dedicated person for reviews

Non-goal:
This sprint should focus on getting the code to run with clean flake8. In the process many places for code improvements can be detected. In order to keep it easy for reviewers I propose to not fix or clean surrounding code unless this is introduced by the flake8/autopep8 change or helps for that and is sufficiently small (must be visibly correct on first glance).
Otherwise the amount of changes will be to much to handle for the reviewer.

Tasks

easybuild-framework repo (317 warnings)

easybuild-easyblocks repo (562 warnings)

=> assigned to @Flamefire

  • work alphabetically, top to bottom (a -> z)

  • please open PRs per letter subdir, for easy testing with --include-easyblocks-from-pr

  • @wpoely86 and @boegel can help out after tackling easybuild-framework, and work backwards (from z -> a)

  • easybuild/easyblocks (547 warnings)

    • easybuild/easyblocks/__init__.py (1 warnings)
    • easybuild/easyblocks/0 (no warnings)
    • easybuild/easyblocks/a (46 warnings)
    • easybuild/easyblocks/b (18 warnings)
    • easybuild/easyblocks/c (29 warnings)
    • easybuild/easyblocks/d (59 warnings)
    • easybuild/easyblocks/e (14 warnings)
    • easybuild/easyblocks/f (16 warnings)
    • easybuild/easyblocks/g (31 warnings)
    • easybuild/easyblocks/generic (12 warnings)
    • easybuild/easyblocks/h (11 warnings)
    • easybuild/easyblocks/i (16 warnings)
    • easybuild/easyblocks/j (0 warnings)
    • easybuild/easyblocks/k (0 warnings)
    • easybuild/easyblocks/l (18 warnings)
    • easybuild/easyblocks/m (56 warnings)
    • easybuild/easyblocks/n (54 warnings)
    • easybuild/easyblocks/o (10 warnings)
    • easybuild/easyblocks/p (27 warnings)
    • easybuild/easyblocks/q (2 warnings)
    • easybuild/easyblocks/r (21 warnings) => @wpoely86
    • easybuild/easyblocks/s (42 warnings) => @wpoely86
    • easybuild/easyblocks/t (19 warnings) => @wpoely86
    • easybuild/easyblocks/{u,v} (2 warnings) => @wpoely86
    • easybuild/easyblocks/w (42 warnings) => @wpoely86
    • easybuild/easyblocks/x (no warnings)
    • easybuild/easyblocks/y (no warnings)
    • easybuild/easyblocks/z (no warnings)
  • setup.py (2 warnings) => @wpoely86

  • test/ (14 warnings) => @wpoely86

easybuild-easyconfigs repo (36 warnings)

easybuild repo (30 warnings)

@boegel boegel added this to the next release (4.2.3?) milestone Aug 18, 2020
@boegel
Copy link
Member

boegel commented Aug 18, 2020

Agreed on the last point, when such stuff comes into view, please open an issue to follow up on it!

@boegel
Copy link
Member

boegel commented Apr 10, 2021

We're running flake8 in CI in all easybuild-* repos now, so closing this...

@boegel boegel closed this as completed Apr 10, 2021
@boegel boegel modified the milestones: next release (4.3.4?), 4.x Apr 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants