-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Make linting failure message more clear #34497
Conversation
Then maybe remove "Formatted foo.json" line? It only confuses, as for me. |
53d026a
to
4c1a279
Compare
Right now when a json file is not linted, the program just says 'Formatted $FILENAME' and exits - this doesn't make it particularly clear what is wrong when a file is not linted. This aims to make it more clear for new contributors.
Done. |
@@ -203,7 +203,7 @@ int main( int argc, char *argv[] ) | |||
std::ofstream fout( filename, std::ios::binary | std::ios::trunc ); | |||
fout << out.str(); | |||
fout.close(); | |||
std::cout << "Formatted " << filename << std::endl; | |||
std::cout << filename << " needs to be linted" << std::endl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding
Not doing this, as it can indicate that the contributor hasn't read the JSON style guide.
Might I suggest explicitly mentioning this in the failure text? Some users may not even know the style guide exists.
std::cout << filename << " needs to be linted" << std::endl; | |
std::cout << filename << " needs to be linted" | |
"\nPlease read doc/JSON_STYLE.md" << std::endl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I didn't see this, sorry.
I don't have any problems doing it
Though I would have done
std::cout << filename << " needs to be linted" << std::endl; | |
std::cout << filename << " needs to be linted" << std::endl; | |
std::cout << "Please read doc/JSON_STYLE.md" << std::endl; |
If you'd like to PR it, feel free.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries. And fair enough, that is way cleaner.
To inform new contributors of the existance of the style guide expands on CleverRaven#34497
Summary
SUMMARY: None
Purpose of change
Make it more clear what is wrong when a file is not linted.
Describe the solution
Add a line that says '$FILENAME needs to be linted' to the output when a file is not linted.
Describe alternatives you've considered
Not doing this, as it can indicate that the contributor hasn't read the JSON style guide.