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

Always run all formatting steps #189

Closed
wants to merge 1 commit into from
Closed

Always run all formatting steps #189

wants to merge 1 commit into from

Conversation

ericof
Copy link
Member

@ericof ericof commented Nov 10, 2023

Add ignore_errors and ignore_outcome to [testenv:format] in tox to always run all formatting steps

Fixes #188

@ericof ericof added the 04 type: enhancement making existing stuff better label Nov 10, 2023
@gforcada
Copy link
Member

I was wondering when someone would report this...

That's a very conscious design decision 🎓

Rationale

If you are formatting a project for the first time, you don't want isort, black, pyupgrade and what not to modify the code in all sorts of ways. Specially if you are the one having to review that PR 🤦🏾

Instead, by stopping at each formatter, you allow yourself to make:

tox -e format
git add -A
git commit -m"chore: pyupgrade"

tox -e format
git add -A
git commit -m"chore: isort"

tox -e format
git add -A
git commit -m"chore: black"

tox -e format
git add -A
git commit -m"chore: zpretty"

That makes you a great and tidy developer that cares about the others' time to review your PRs ✨

Backdoor 🚪

If you don't care about the above, simply run tox -e lint that will format AND lint the code all in a single step

That's why we have two tox environments (format/lint) so we can use either approach.

I personally, when configuring plone/meta for the first time on a project I use tox -e format and do the proper commits, and afterwards use tox -e lint.

When producing patches, I mostly use directly tox -e lint as the base branch is already ensured to be tox -e format complaint 🤖

Only when doing a major refactoring, I tend to use again tox -e format

Does that make sense? 🤔

At the end, it all depends on what you use in CI... if on CI you already are already running tox -e lint, then you can simply ignore tox -e format. If you only care about tox -e format on CI, then the linting might be too verbose/noisy for you ⚖️

@mauritsvanrees
Copy link
Member

I agree with Gil here.
Also, you can run pre-commit run -a without using tox.

@ericof
Copy link
Member Author

ericof commented Nov 11, 2023

@gforcada I see your points :-)

My use case is not for reformatting but running it after a new package is created with Cookiecutter. It makes no sense to create many commits on a newly created codebase :-)

Of course, in a large codebase, your point is 100% valid during a reformat.

Anyway, let me test running just the lint after the code creation

@gforcada gforcada closed this Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
04 type: enhancement making existing stuff better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tox -e format should not stop on errors
3 participants