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

api: Lint metadata.py #1294

Closed

Conversation

rzr
Copy link
Contributor

@rzr rzr commented Mar 2, 2021

While working using pylint 2.5.3 (from debian package 2.5.3-2)
I noticed those warnings that were not triggered
by current tox configuration.

This will not harm to support several pylint version.

Change-Id: I7f4d5d56a157cab27f19cfe74cd74baa893c1d93
Signed-off-by: Philippe Coval [email protected]

Please fill in the fields below to submit a pull request. The more information
that is provided, the better.

Fixes #

Description of the changes being introduced by the pull request:

Please verify and check that the pull request fulfills the following
requirements
:

  • The code follows the Code Style Guidelines
  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

@joshuagl
Copy link
Member

joshuagl commented Mar 3, 2021

Thank you for the PR Philippe. These changes look appropriate. I wonder why the current tox configuration did not pick up these issues, it seems we are perhaps not properly listing this file?

@rzr
Copy link
Contributor Author

rzr commented Mar 3, 2021

Ok I investigated a bit it looks like latest pylint 2.7.2 removed this "bad-continuation" feature

pylint-dev/pylint#3571

while it was still present in earlier version (2.5.3 at least)

Since 2.6 upstream suggests to use black formater:

https://github.com/PyCQA/pylint/blob/4faccd0563c628954f04322ffb696905a3dab62d/doc/whatsnew/2.6.rst

Any objection to use black for formatting ?:
#1178

Nevertheless is this change still desirable?
I will update commit message and refresh this PR.

rzr added a commit to CrossStream/tuf that referenced this pull request Mar 3, 2021
While working using pylint 2.5.3 (from debian package 2.5.3-2)
I noticed those warnings that were not triggered
by pylint 2.7.2 used in current tox configuration.

After some investigation pylint-2.6,
dropped "bad-continuation" feature.

This will not harm to support older pylint versions,
until an other formater (like blank) is introduced.

Relate-to: theupdateframework#1294
Signed-off-by: Philippe Coval <[email protected]>
@rzr rzr force-pushed the sandbox/rzr/review/master branch from b9d894e to 9e47333 Compare March 3, 2021 13:56
While working using pylint 2.5.3 (from debian package 2.5.3-2)
I noticed those warnings that were not triggered
by pylint 2.7.2 used in current tox configuration.

After some investigation pylint-2.6,
dropped "bad-continuation" feature.

This will not harm to support older pylint versions,
until an other formater (like black) is introduced.

Relate-to: theupdateframework#1294
Signed-off-by: Philippe Coval <[email protected]>
@rzr rzr force-pushed the sandbox/rzr/review/master branch from 9e47333 to af9c14f Compare March 3, 2021 13:58
@lukpueh
Copy link
Member

lukpueh commented Mar 8, 2021

Nevertheless is this change still desirable?

Yes! But maybe we can defer this PR until we have a proper linter/auto-formatter config (#1178)? And then have a flag day, where we make all changes, grouping them thematically by commit? What do you think?

@rzr
Copy link
Contributor Author

rzr commented Mar 9, 2021

Hi @lukpueh , sure this can be delayed since it's not critical at all.

May i suggest this change to be merged for next version if we fail to setup linter before releasing

I'll see how can I help with black:
#1178

@lukpueh
Copy link
Member

lukpueh commented Mar 17, 2021

Superseded by #1314.

@lukpueh
Copy link
Member

lukpueh commented Mar 17, 2021

Thanks for the getting the ball rolling, @rzr!

@lukpueh lukpueh closed this Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants