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

Combine --check and --dry-run into a single option. #541

Merged
merged 4 commits into from
Feb 14, 2020

Conversation

JoeRobich
Copy link
Member

The use case for --check (CI Validation) also uses --dry-run. Use cases for --dry-run could ignore exit code if formatting changes are found.

Considers --check as the canonical flag and --dry-run as an alias.

@JoeRobich JoeRobich requested a review from jmarolf February 13, 2020 18:15
@jmarolf
Copy link
Contributor

jmarolf commented Feb 13, 2020

I approve of this change. My only question is "should we bump the version number as this is a breaking change?"

@JoeRobich
Copy link
Member Author

@jmarolf I plan to bump version numbers for each release. That being said. I should start doing it after a release...

@JoeRobich
Copy link
Member Author

Version bump in this PR #543

@jmarolf
Copy link
Contributor

jmarolf commented Feb 13, 2020

Shouldn't this be a major version bump though?

@JoeRobich
Copy link
Member Author

@jmarolf sure

@JoeRobich JoeRobich merged commit 032c515 into dotnet:master Feb 14, 2020
@xt0rted
Copy link

xt0rted commented Mar 3, 2020

I have a GitHub Action that wraps this tool (xt0rted/dotnet-format) and I'm about to release it but was wondering how this change will effect my usage of it first.

When linting I use --check --dry-run so I can let the user decide if the workflow should exit with an error or not, and when applying fixes I'm also using --check to know if the rest of the workflow should continue (committing & pushing changes back).

Now that --check and --dry-run have been merged is there a way for the caller to know if files were formatted? I guess I could add another step that uses git for this, but it'd be easier if the tool told me instead.

@JoeRobich
Copy link
Member Author

Hi @xt0rted, Our understanding was that --check and --dry-run were used together basically 100% of the time for the purpose of linting (once users realized they needed to have both). I would recommend checking git for unstaged changes if you are relying on the old behavior of --check. Sorry again for the change.

@JoeRobich JoeRobich deleted the unify-check-dry-run branch March 5, 2021 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants