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

Fix pylint warnings #1584

Merged
merged 2 commits into from
Sep 17, 2021
Merged

Fix pylint warnings #1584

merged 2 commits into from
Sep 17, 2021

Conversation

MVrachev
Copy link
Collaborator

@MVrachev MVrachev commented Sep 17, 2021

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

Description of the changes being introduced by the pull request:

New pylint warnings appeared (for the first time here: https://github.com/theupdateframework/python-tuf/pull/1446/checks?check_run_id=3630842220) with code:
W0602: Using global for X but no assignment is done (global-variable-not-assigned)
where X is a global variable.
In the lines pointed by the warnings, "global" was used inside a
function scope in order to enable assignment of the global variable to
a new value in the function.
The reason for these warnings was that pylint noticed we are using
"global" for the variable X inside a function scope without actually
assigning it to a new value.
Overall that was true for the reported cases and I removed the use of
"global", but there were only two cases where pylint reported a
false-positive. Then we were using "global", but pylint didn't
understand there were assignments.
In those cases, I disabled the warnings.

Signed-off-by: Martin Vrachev mvrachev@vmware.com

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

@coveralls
Copy link

coveralls commented Sep 17, 2021

Pull Request Test Coverage Report for Build 1246105286

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 98.162%

Totals Coverage Status
Change from base Build 1245173711: 0.0%
Covered Lines: 3792
Relevant Lines: 3829

💛 - Coveralls

Copy link
Member

@jku jku left a comment

Choose a reason for hiding this comment

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

I looked at a few cases and pylint is correct: those globals are not needed.

global is only required if you want to assign to the global variable. mutating the value of that variable does not require global.

@jku
Copy link
Member

jku commented Sep 17, 2021

https://www.programiz.com/python-programming/global-keyword

these third party documentation sites have really variable quality... The whole point of global is that assignment to global variables can only work with global, yet this is not mentioned at all.

I try to find references for this sort of things on python.org (the search engines really fail hard with python searches, always offering this sort of pages before python.org content)

@MVrachev MVrachev force-pushed the fix-warning branch 2 times, most recently from d4b6632 to 47eee7c Compare September 17, 2021 15:49
New pylint warnings appeared with code "W0602: Using global for X but
no assignment is done (global-variable-not-assigned)" where X is a
global variable.
In the lines pointed by the warnings, "global" was used inside a
function scope in order to enable assignment of the global variable to
a new value in the function.
The reason for these warnings was that pylint noticed we are using
"global" for the variable X inside a function scope without actually
assigning it to a new value.
Overall that was true for the reported cases and I removed the use of
"global", but there were only two cases where pylint reported a
false-positive. Then we were using "global", but pylint didn't
understand there were assignments.
In those cases, I disabled the warnings.

Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
@MVrachev
Copy link
Collaborator Author

I looked at a few cases and pylint is correct: those globals are not needed.

global is only required if you want to assign to the global variable. mutating the value of that variable does not require global.

Updated the pr and removed all but two occurrences of global which are false-positives.

these third party documentation sites have really variable quality... The whole point of global is that assignment to global variables can only work with global, yet this is not mentioned at all.

Yes, you are right. Removed the link from the commit message and pr description.

I try to find references for this sort of things on python.org (the search engines really fail hard with python searches, always offering this sort of pages before python.org content)

I can't find where on python org globals are mentioned and that's why I won't give a link.

Simplify the code by removing some of the uses of "global" by using
alternativies to the assignements.

Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Copy link
Member

@jku jku left a comment

Choose a reason for hiding this comment

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

-55 lines, fixes CI

Nice

@jku jku merged commit c5e0bdd into theupdateframework:develop Sep 17, 2021
@MVrachev MVrachev deleted the fix-warning branch September 20, 2021 11:08
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