Return failure exit code on found diffs (fix #906) #1075
Conversation
|
@nrc r? I wasn't sure about a good way to test this, did I overlook something? |
|
cc @matklad to check this doesn't break any of your uses |
src/summary.rs
Outdated
| // Code is valid, but it is impossible to format it properly. | ||
| has_formatting_errors: bool, | ||
|
|
||
| pub diff_result: i32, |
There was a problem hiding this comment.
Could you add a comment for this field
Ah, we've not yet implemented tight rustfmt integration, so nothing will break on our side :( (But our own formatter is improving by leaps and bounds). However, I am not sure that using the number of errors as an exit code is the best possible approach. I'd rather simply return zero or one. If tools need to know what exactly is the number of differences, they'd better parse the output of the diff command. |
Can you elaborate why? |
|
The main reason is that if Some other concerns are:
|
|
The related PR is #902 |
|
Ok, that seems reasonable. I'll change it to, uh, 4. :) |
src/summary.rs
Outdated
| pub fn has_diff(&self) -> bool { | ||
| self.has_diff | ||
| } | ||
|
|
There was a problem hiding this comment.
we don't need this method, just make the field public
There was a problem hiding this comment.
yeah, good point, wanted to stay consistent, but I guess it's really unnecessary
|
@johannhof thanks for the change. I just have one last comment, then r+. |
This changes rustfmt to return exit code 4 when run with write mode diff and differences between the formatted code and the original code are found. Useful for CI to make sure your contributors actually ran rustfmt.
|
@nrc done |
|
Thanks! |
3 participants
This changes rustfmt to return the number
of found differences as exit code when run with
write mode diff.
Useful for CI to make sure your contributors actually ran rustfmt.