Skip to content

Conversation

@boegel
Copy link
Member

@boegel boegel commented Aug 19, 2020

wpoely86
wpoely86 previously approved these changes Aug 20, 2020
run: |
flake8 easybuild/tools
flake8 test/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not change this during the sprint. My suggestion is to enable this at the very end for .. Otherwise all other PRs will conflict when changing this too.
Oh and according to GH it is missing the final newline and the trailing slash is not required :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, I agree.

But in this case, it's fine to leave this in if @wpoely86 bases his work for the easybuild folder on top of this PR (all the changes there can be made in one go, and then linting.yml can be changed to use flake8 ., and we're done?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure if you want to make those dependent PRs but IMO it is easier to verify locally via flake8 and don't change this yet.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For easyblocks repo, I definitely agree we should hold off touching (actually adding) linting.yml until we're done.

@hound hound bot deleted a comment from boegel Aug 20, 2020
@hound hound bot deleted a comment from Flamefire Aug 20, 2020
@wpoely86 wpoely86 merged commit 726d722 into easybuilders:develop Aug 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants