x1ddos
commented
4 years ago +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
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 https://github.com/golang/go/issues/24230 for a similar idea).Happy to send a PR for option 2, if likely to be merged. Thanks.