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

Add option to log modified files #24

Closed
tweksteen opened this issue Dec 18, 2019 · 3 comments
Closed

Add option to log modified files #24

tweksteen opened this issue Dec 18, 2019 · 3 comments

Comments

@tweksteen
Copy link
Member

This is a feature request: for a caller, it would be good to know if the execution modified any file (e.g., as part of a presubmit test). One option is to change the exit code if at least one file has been modified. Another option (I would recommend) is to add an option (for instance, -l) which will print all the filenames that have been modified (see discussion at golang/go#24230 for a similar idea).

Happy to send a PR for option 2, if likely to be merged. Thanks.

@x1ddos
Copy link
Contributor

x1ddos commented Dec 18, 2019

+1 to option 2, a flag. only I would make it -v, as in "verbose". WDYT? Happy to review a change.

Re: exit code. It definitely makes sense for gofmt because it's always clear what the desired outcome is: code is formatted appropriately.

But here it isn't as clear what the desire is imho. Both "no files modified" and "at least one file modified" can be an ok expectation. Additionally, it would be confusing when addlicense exits with non-zero code due to say an I/O error.

We could of course add and document different exit codes for different scenarios but it still isn't clear when you're just checking for non-zero.

Also, nothing prevents us from adding both exit codes and a flag but I would definitely start with the latter and see whether it's sufficient.

@tweksteen
Copy link
Member Author

Agreed. Sent #25 for review.

@x1ddos
Copy link
Contributor

x1ddos commented Jan 25, 2020

This is resolved by #25

@x1ddos x1ddos closed this as completed Jan 25, 2020
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

2 participants