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

Command line option to check whether pyproject.toml would be reformatted #10

Closed
kieran-ryan opened this issue Jun 29, 2023 · 6 comments
Closed
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@kieran-ryan
Copy link
Owner

Like other Python formatters such as black and isort, having a command line option to check whether formatting would be applied - without actually applying formatting - would be useful for linting purposes, such as in a continuous integration pipeline, pre-commit git hook, or to check locally from the command line whether it is formatted correctly.

pyprojectsort --check
@kieran-ryan kieran-ryan added enhancement New feature or request good first issue Good for newcomers labels Jun 29, 2023
@kieran-ryan kieran-ryan changed the title Command line option to check whether pyproject.toml would be formatted Command line option to check whether pyproject.toml would be reformatted Jul 1, 2023
@DariaZvereva
Copy link
Contributor

@kieran-ryan Hi, I found this repo on https://goodfirstissues.com
I am looking for some good first issues and this looks perfect. Do you mind if I try to solve it?

May be it should be a solution for #16 as you mentioned and also #12. Do you have any thoughts about it?

@kieran-ryan
Copy link
Owner Author

kieran-ryan commented Jul 1, 2023

Hey @DariaZvereva, yes, please! Go for it! 😃 If you have any questions, just let me know.

I've just implemented #12, so there's code to check whether reformatting will occur or not. My thoughts for the next steps are below, however I welcome you to think of the best way to implement this:

  • Add a --check command line option
  • Output a message such as f"'{args.file}' would be reformatted" and terminate with exit code 1 if the check option is used and reformatting would occur; and otherwise to output a f"'{args.file}' would be left unchanged" message if the check option is set but reformatting would not occur (i.e. no changes)
  • Add unit tests covering the new behaviour
  • Document the option in the README file

The 'diff' option (#16) might be something to take into consideration while implementing this, but can be tackled separately or fixed with this issue; though separately might be easier for issue resolution - but will leave that up to you.

Just a note as well that once this is implemented and a new release is created from GitHub (pushing it to PyPI), I then plan to run the package against its own pyproject.toml and add the check option as a linting task in the GitHub workflow pipeline - ensuring validation - the same way I would expect users might use this feature. It may also be useful for the pre-commit support (#13).

I also welcome your contributions, ideas, bugs and feedback; so feel free to create an Issue if anything you think should be implemented or if there's anything that you would really like to implement yourself. Thanks for your interest in the project! 👍

@DariaZvereva
Copy link
Contributor

terminate with exit code 1 - I think it's better to exit with 0 code, because it's not an error, just successful termination. And 0 is more common way to tell that run was successful.

@kieran-ryan
Copy link
Owner Author

kieran-ryan commented Jul 1, 2023

terminate with exit code 1 - I think it's better to exit with 0 code, because it's not an error, just successful termination. And 0 is more common way to tell that run was successful.

I would have shared the same opinion; though taking a look at other formatters, such as black (below), they appear to output 1 as it can then be run from a pipeline/hook and indicate a failure though not reformat the file e.g. if I was to run pyprojectsort in a pipeline, it would fail the job as expected, but it would also modify the file which may affect follow on jobs (such as linters) as it would no longer be a clean clone of the repo. With a pyprojectsort --check option that outputs 1 if reformatting would be applied, I can fail the job but the cloned repo stays clean for other jobs to run against. What do you think?

image

@DariaZvereva
Copy link
Contributor

With a pyprojectsort --check, I can fail the job but the cloned repo stays clean for other jobs to run against. What do you think?

Ok, now this makes sense to me:) Let's keep a consistent behaviour across different formatters.

@kieran-ryan
Copy link
Owner Author

Closed by PR #20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
Status: Done 🙌
Development

No branches or pull requests

2 participants