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

Yor could use a check mode (see PR#364) #366

Closed
hartzell opened this issue May 15, 2023 · 6 comments
Closed

Yor could use a check mode (see PR#364) #366

hartzell opened this issue May 15, 2023 · 6 comments

Comments

@hartzell
Copy link
Contributor

A while back, in #254, I suggested that a dry-run mode would yor more useful, because I wanted to:

write a test that calls yor --check or yor --dry-run and examine its exit code.

@hi-artem implemented the "don't actually change anything" side of the feature in #257, but it does not do anything useful with the exit status so we're left with parsing the CLI output.

I've submitted #364, which implements a check mode, which causes the output to be 0 if nothing changed and non-zero (via log.Error if anything changed (would have changed, if --dry-mode).

Feedback welcome.

@omryMen
Copy link
Collaborator

omryMen commented May 16, 2023

hey @hartzell,
the problem with changing the exit code is that you won't know if a real error occurred in the code which can be confusing.
if going for it, it makes sense to do only when running dry mode and not in regular run.
as for naming I would prefer calling it validation instead of check

@hartzell
Copy link
Contributor Author

I don't see confusion being a big issue, since all of the exiting (including mine) is done through log.Error, which prints a message explaining what went wrong.

Happy to connect it to dry-run though, that was my original intent but assumed that people didn't want it there.

I've updated the PR.

@omryMen
Copy link
Collaborator

omryMen commented May 17, 2023

@hartzell what I meant is not to unite the flags. but when adding the new one you've suggested - make it turn on the dry mode as well, that way you will assure that this mode will have no side affects and only validate that there are no changes to be made

@hartzell
Copy link
Contributor Author

@omryMen -- done.

@stale
Copy link

stale bot commented Jun 17, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Jun 17, 2023
@hartzell
Copy link
Contributor Author

Closed by #364.

@ChanochShayner ChanochShayner removed the wontfix This will not be worked on label Aug 28, 2023
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

No branches or pull requests

3 participants