Upgrade and broaden flake8, fixing style problems and bugs by EliahKagan · Pull Request #1673 · gitpython-developers/GitPython

added 5 commits

September 20, 2023 22:32
Upgrading flake8 from 6.0.0 to 6.1.0 causes its pycodestyle
dependency to be upgraded from 2.10.* to 2.11.*, which is desirable
because:

- Spurious "E231 missing whitespace after ':'" warnings on 3.12 due
  to the lack of full compatibility with Python 3.12 are gone.

- New warnings appear, at least one of which, "E721 do not compare
  types, for exact checks use `is` / `is not`, for instance checks
  use `isinstance()`", does identify something we can improve.

This upgrades flake8 in pre-commit and fixes the new warnings.
This bumps the versions of the flake8 plugins specified with pinned
versions as additional dependencies of flake8 for pre-commit.

Doing so gains a warning about a call to warnings.warn with no
stacklevel argument. This appears to be the uncommon case where the
implifit effect of stacklevel=1 is intended, so I have made that
explicit, which clarifies this intent and dismisses the warning.
This expands flake8 linting to include the test suite, and fixes
the resulting warnings. Four code changes are especially notable:

- Unit tests from which documentation is automatically generated
  contain a few occurrences of "# @NoEffect". These suppressions
  are extended to "# noqa: B015  # @NoEffect" so the corresponding
  suppression is also applied for flake8. This is significant
  because it actually changes what appears in the code examples in
  the generated documentation. However, since the "@NoEffect"
  annotation was considered acceptable, this may be okay too. The
  resulting examples did not become excessively long.

- Expressions like "[c for c in commits_for_file_generator]" appear
  in some unit tests from which documentation is automatically
  generated. The simpler form "list(commits_for_file_generator)"
  can replace them. This changes the examples in the documentation,
  but for the better, since that form is simpler (and also a better
  practice in general, thus a better thing to show readers). So I
  made those substitutions.

- In test_repo.TestRepo.test_git_work_tree_env, the code intended
  to unpatch environment variables after the test was ineffective,
  instead causing os.environ to refer to an ordinary dict object
  that does not affect environment variables when written to. This
  uses unittest.mock.patch.dict instead, so the variables are
  unpatched and subsequent writes to environment variables in the
  test process are effective.

- In test_submodule.TestSubmodule._do_base_tests, the expression
  statement "csm.module().head.ref.tracking_branch() is not None"
  appeared to be intended as an assertion, in spite of having been
  annoated @NoEffect. This is in light of the context and because,
  if the goal were only to exercise the function call, the
  "is not None" part would be superfluous. So I made it an
  assertion.
This refactors code in the test suite that uses try-finally, to
simplify and clarify what it is doing. An exception to this being
a refactoring is that a possible bug is fixed where a throwaway
environment variable "FOO" was patched for a test and never
unpatched.

There are two patterns refactored here:

- try-(try-except)-finally to try-except-finally. When the entire
  suite of the try-block in try-finally is itself a try-except,
  that has the same effect as try-except-finally. (Python always
  attempts to run the finally suite in either case, and does so
  after any applicable except suite is run.)

- Replacing try-finally with an appropriate context manager.

(These changes do not fix, nor introduce, any flake8 errors, but
they were found by manual search for possible problems related to
recent flake8 findings but outside of what flake8 can catch. To
limit scope, this only refactors try-finally in the test suite.)
This clarifies that the object's contents are patched, rather than
patching the attribute used to get the object in the first place.
It is also in keeping with the style of patching os.environ
elsewhere in the test suite.

This was referenced

Sep 21, 2023

renovate bot referenced this pull request in allenporter/flux-local

Sep 23, 2023

otc-zuul bot pushed a commit to opentelekomcloud-infra/eyes_on_docs that referenced this pull request

Oct 25, 2023
Bump gitpython from 3.1.35 to 3.1.37

Bumps gitpython from 3.1.35 to 3.1.37.

Release notes
Sourced from gitpython's releases.

3.1.37 - a proper fix CVE-2023-41040
What's Changed

Improve Python version and OS compatibility, fixing deprecations by @​EliahKagan in gitpython-developers/GitPython#1654
Better document env_case test/fixture and cwd by @​EliahKagan in gitpython-developers/GitPython#1657
Remove spurious executable permissions by @​EliahKagan in gitpython-developers/GitPython#1658
Fix up checks in Makefile and make them portable by @​EliahKagan in gitpython-developers/GitPython#1661
Fix URLs that were redirecting to another license by @​EliahKagan in gitpython-developers/GitPython#1662
Assorted small fixes/improvements to root dir docs by @​EliahKagan in gitpython-developers/GitPython#1663
Use venv instead of virtualenv in test_installation by @​EliahKagan in gitpython-developers/GitPython#1664
Omit py_modules in setup by @​EliahKagan in gitpython-developers/GitPython#1665
Don't track code coverage temporary files by @​EliahKagan in gitpython-developers/GitPython#1666
Configure tox by @​EliahKagan in gitpython-developers/GitPython#1667
Format tests with black and auto-exclude untracked paths by @​EliahKagan in gitpython-developers/GitPython#1668
Upgrade and broaden flake8, fixing style problems and bugs by @​EliahKagan in gitpython-developers/GitPython#1673
Fix rollback bug in SymbolicReference.set_reference by @​EliahKagan in gitpython-developers/GitPython#1675
Remove @NoEffect annotations by @​EliahKagan in gitpython-developers/GitPython#1677
Add more checks for the validity of refnames by @​facutuesca in gitpython-developers/GitPython#1672

Full Changelog: gitpython-developers/GitPython@3.1.36...3.1.37



Commits

b27a89f fix makefile to compare commit hashes only
0bd2890 prepare next release
832b6ee remove unnecessary list comprehension to fix CI
e98f57b Merge pull request #1672 from trail-of-forks/robust-refname-checks
1774f1e Merge pull request #1677 from EliahKagan/no-noeffect
a4701a0 Remove @NoEffect annotations
d40320b Merge pull request #1675 from EliahKagan/rollback
d1c1f31 Merge pull request #1673 from EliahKagan/flake8
e480985 Tweak rollback logic in log.to_file
ff84b26 Refactor try-finally cleanup in git/
Additional commits viewable in compare view




Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

@dependabot rebase will rebase this PR
@dependabot recreate will recreate this PR, overwriting any edits that have been made to it
@dependabot merge will merge this PR after your CI passes on it
@dependabot squash and merge will squash and merge this PR after your CI passes on it
@dependabot cancel merge will cancel a previously requested merge and block automerging
@dependabot reopen will reopen this PR if it is closed
@dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
@dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
@dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
@dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
@dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
You can disable automated security fix PRs for this repo from the Security Alerts page.

Reviewed-by: Vladimir Vshivkov