Update readme and tox.ini for recent tooling changes#1868
Merged
Byron merged 4 commits intogitpython-developers:mainfrom Mar 13, 2024
Merged
Update readme and tox.ini for recent tooling changes#1868Byron merged 4 commits intogitpython-developers:mainfrom
Byron merged 4 commits intogitpython-developers:mainfrom
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There are some changes that I should have proposed and/or made in
README.mdandtox.inirelated to #1862, which are made slightly more needed by #1865. This brings the readme and tox configuration up to date for both #1862, which was merged, and #1865, which I think will be merged soonpending one small change. Further related changes should be eventually be made in both, but they will depend on future decisions, and nothing bad will happen if changes end up not being made for an extended time after merging both #1865 and this PR.The changes here seem significantly more cumbersome to recommend be included in #1865 than to do separately, and because they also apply to the situation brought about since #1862, I don't think they need to be done there or done before that is merged. I suggest #1865 be merged before this; the changes here will not be all correct until #1865 comes in anyway. #1865 could be merged and then this immediately merged, but I think it is also fine to merge #1865 and request changes here; that is, I think #1865 is reasonable to merge even if this is delayed. Merging them in the other order would also be okay, I think.
The core issue is that some actions that were presented as not changing file contents now change them. Currently on the main branch:
toxwith no arguments is always in practice assumed not to change source code in the working tree. But itslintenvironment changes source code due to runningruffwith--fixthroughpre-commit. With #1865, this will happen in more cases, but the issue already exists. As noted below and in commit messages, I believe--fixis nonetheless good and should be kept. (When I reviewed #1862 I did not mention this, because I agreed with the change, which is what users ofpre-commitwill expect. In hindsight, I could've simplified things by proposing these changes at that time.)make lintas linting and running a formatting check without modifying any files. This is likewise not fully accurate since #1862. The issue is not thatmake lintneeds to behave differently, because (a) in hindsight I shouldn't have added that target, (b) I'm not sure anyone uses it, and (c) it was added to address a situation where checking for code style and formatting problems involved multiple commands and tool and plugin packages, instead of justruff. So it's sufficient for the readme to stop recommending it as a way to avoid linting without automatic code changes.This PR fixes (1) by making the tox
lintenvironment not run unless explicitly listed on the command line, for now, and fixes (2) by changing the readme. This also fixes some related outdated material in the readme, improves how tools are described, and makes some some other small improvements when it seemed like I could do them without significantly complicating this PR or its review.Most information about these changes is in the two most important commits, c66257e and 91f967a. This includes some information about why I believe
--fixshould be kept.