lint: switch Black with ruff-format by Borda · Pull Request #1865 · gitpython-developers/GitPython (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Borda marked this pull request as ready for review

March 11, 2024 20:36

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Mar 12, 2024

• make lint no longer lints without making changes. This has been the case since gitpython-developers#1862, because --fix is used. (I don't think anyone was really using make lint, but just in case, we should avoid saying it still never changes any files.) With gitpython-developers#1865, there will be more ways that files can be changed.

• black is no longer used, having been replaced by ruff. This will be the case as of gitpython-developers#1865.

This also reorganizes the "Specific tools" list, since they are all configured in pyproject.toml now (only flake8 was not before, and it was removed in favor of ruff in gitpython-developers#1862). In doing so, I've also added brief parenthesized phrases to characterize what each of these four tools is for, so readers don't have to look around as much to understand most of the tooling GitPython has set up.

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Mar 12, 2024

This is to make it so simple tox usage has the expected property of leaving all source code files in the working tree unchanged.

As noted in c66257e, linting how sometimes performs auto-fixes since gitpython-developers#1862, and the pre-commit command in tox.ini, which had also run black --check, will do even more file editing with gitpython-developers#1865.

The bifurcation for black into separate mutating and non-mutating hooks, introduced in 5d8ddd9 (gitpython-developers#1693), is not carried over into Ruff autoformatting in gitpython-developers#1865. But also it:

• Was not necessarily a good approach, and likely should not be preserved in any form. It is an unusual and unintuitive use of pre-commit. (It can be brought back if no better approach is found, though.)

• Was done to avoid a situation where it was nontrivial to set up necessary dependencies for linting in the GitPython virtual environment itself, because flake8 and its various plugins would have to be installed.

  They were not listed in any existing or newly introduced extra (for example, they were not added to test-requirements.txt) in part in the hope that they would all be replaced by Ruff, which happened in gitpython-developers#1862.

• Already does not achieve its goal since gitpython-developers#1862, since it was (probably rightly) not extended to Ruff linting to use/omit --fix.

Now that Ruff is being used, people can run pip install ruff in a virtual environment, then run the ruff command however they like. This takes the place of multiple tools and plugins.

This commit avoids doing any of the following, even though it may be useful to do them later:

• This does not give specific instructions in the readme for installing and running ruff (and c66257e before this also omits that). This can be added later and the best way to document it may depend on some other upcoming decisions (see below).

• This does not add ruff to the test extra or as any other kind of extra or optional dependency. Although the test extra currently contains some packages not used for running unit tests, such as pre-commit and mypy, adding Ruff will cause installation to take a long time and/or or fail on some platforms like Cygwin where Ruff has to be built from (Rust) source code. This can be solved properly by reorganizing the extras, but that is likely to wait until they are expressed completely inside pyproject.toml rather than one per requirements file (see discussion in comments in gitpython-developers#1716 for general information about possible forthcoming changes in how the project is defined).

• This does not update tox.ini to run ruff directly, which could be done by splitting the current lint tox environment into two or three environments for Python linting, Python autoformatting, and the miscellaneous other tasks performed by pre-commit hooks, only the latter of which would use the pre-commit command. Whether and how this should be done may depend on other forthcoming changes.

• This does not remove or update the Makefile "lint" target. That target should perhaps not have been added, and was always meant to be improved/replaced, since everything else in the top-level Makefile is for building and publishing releases. See 5d15063 (gitpython-developers#1693). This is likewise not done now since it may depend on as-yet unmerged changes and tooling decisions not yet made. It should be feasible to do together when further updating tox.ini.

• This does not update tox.ini, Makefile, or the lint.yml GitHub Actions workflow to omit the manual hook-stage, which will be unused as of gitpython-developers#1865. This would complicate integration of changes, and once it is known that it won't be needed, it can removed.

The situation with the tox "lint" environment is thus now similar to that of the tox "html" environment when it was added in e6ec6c8 (gitpython-developers#1667), until it was improved in f094909 (gitpython-developers#1693) to run with proper isolation.

Makefile Outdated Show resolved Hide resolved

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Mar 13, 2024

This is to make it so simple tox usage has the expected property of leaving all source code files in the working tree unchanged.

Linting how sometimes performs auto-fixes since gitpython-developers#1862, and the pre-commit command in tox.ini, which had also run black --check, will do even more file editing due to the changes in gitpython-developers#1865.

The bifurcation for black into separate mutating and non-mutating hooks, introduced in 5d8ddd9 (gitpython-developers#1693), was not carried over into Ruff autoformatting in gitpython-developers#1865. But also it:

• Was not necessarily a good approach, and likely should not be preserved in any form. It was an unusual and unintuitive use of pre-commit. (It can be brought back if no better approach is found, though.)

• Was done to avoid a situation where it was nontrivial to set up necessary dependencies for linting in the GitPython virtual environment itself, because flake8 and its various plugins would have to be installed.

  They were not listed in any existing or newly introduced extra (for example, they were not added to test-requirements.txt) in part in the hope that they would all be replaced by Ruff, which happened in gitpython-developers#1862.

• Already did not achieve its goal as of gitpython-developers#1862, since it was (probably rightly) not extended to Ruff linting to use/omit --fix.

Now that Ruff is being used, people can run pip install ruff in a virtual environment, then run the ruff command however they like. This takes the place of multiple tools and plugins.

The situation with the tox "lint" environment is thus now similar to that of the tox "html" environment when it was added in e6ec6c8 (gitpython-developers#1667), until it was improved in f094909 (gitpython-developers#1693) to run with proper isolation.

Borda deleted the ruff/black branch

March 13, 2024 07:14

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Mar 13, 2024

This is no longer used. No pre-commit hook specifies it anymore in stages, since 517f83a (gitpython-developers#1865). See b059cd5 (gitpython-developers#1868) for context.

In the lint.yml GitHub Actions workflow, this removes the extra_args key altogether, because all that would remain there is --all-files, which is already the default for that action, when the extra_args key is absent.

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Mar 13, 2024

Although it seems likely that the requirements-dev.txt file will be removed when the project definition is made declarative (discussed in gitpython-developers#1716 comments), if not before, for now it exists and might be in use, so this updates it with tools that are currently used but not listed in any extras or other requirements files:

• ruff: This has replaced flake8 and its plugins (gitpython-developers#1862) as well as black (gitpython-developers#1865). Currently there is no separate extra for tooling that is not part of unit testing, with some such tools listed in test-requirements.txt. The ruff package belongs here rather than there for now because it should not be installed just to run unit tests, since Ruff has to be built from source on some rarer platforms like Cygwin, which would take a long time and/or fail.

• shellcheck: The PyPI package for this is a convenience for installing it in projects that are already using pip (shellcheck is neither written in Python nor a tool to scan Python code). It installs pre-built binaries, which are not available for all platforms.

These packages remain listed:

• pytest-icdiff: This seems not to have been promoted to be in the test-requirements.txt file and the test extra because it does not work as well for diffs from tests that the pytest runner runs but that are written for the unittest framework.

• pytest-profiling [commented out]: I am not sure what the status of this is, perhaps it has just not been experimented with enough to know if it would be useful for profiling in GitPython.

This requirements-dev.txt file has a few limitations that suggest it should be removed altogether sometime soon:

• It is not updated regularly.

• It is not always clear why something is there. Originally I believe it was for tools where the desire to use the tool was established but the tool did not yet work or worked but performed checks for which code had to be fixed. That purpose has drifted.

• It uses a different naming convention from the test-requirements.txt file in active use.

• It cannot be readily used to create an extra in the current project definition in setup.py because the simple parsing done there will not recognize the -r lines and will not skip the comments, and neither enhancement should be done in setup.py since that would move things farther away from a declarative project definition.

• It will naturally go away when the project definition is made declarative, since it will then be feasible to define as many extras as desired without proliferating separate requirements files (while still allowing their contents to be statically available to tools).

Since it may go away soon and is not regularly updated, I have kept the explanations for why particular packages are there out of it. But as long as it exists it may as well list the tools that really are being used yet are not explicitly listed as dependencies.

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

Mar 31, 2024

](https://renovatebot.com)

This PR contains the following updates:

Package	Change	Age	Adoption	Passing	Confidence
GitPython
==3.1.42 -> ==3.1.43
](https://docs.renovatebot.com/merge-confidence/)
](https://docs.renovatebot.com/merge-confidence/)
](https://docs.renovatebot.com/merge-confidence/)
](https://docs.renovatebot.com/merge-confidence/)

Release Notes

Configuration

📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 Automerge: Enabled.

♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this PR and you won't be reminded about this update again.

• If you want to rebase/retry this PR, check this box

This PR has been generated by Mend Renovate. View repository job log here.

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

lettuce-bot bot referenced this pull request in lettuce-financial/github-bot-signed-commit

Apr 1, 2024

](https://renovatebot.com)

This PR contains the following updates:

Package	Change	Age	Adoption	Passing	Confidence
GitPython
==3.1.42 -> ==3.1.43
](https://docs.renovatebot.com/merge-confidence/)
](https://docs.renovatebot.com/merge-confidence/)
](https://docs.renovatebot.com/merge-confidence/)
](https://docs.renovatebot.com/merge-confidence/)

Release Notes

Configuration

📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this PR and you won't be reminded about these updates again.

• If you want to rebase/retry this PR, check this box

This PR has been generated by Mend Renovate. View repository job log here.