docs: Added information on how to run the linter. #7534
Conversation
| @@ -150,6 +150,12 @@ $ ./configure && make -j8 test | |||
| Make sure the linter is happy and that all tests pass. Please, do not submit | |||
| patches that fail either check. | |||
|
|
|||
| You can check if the linter is happy by running: | |||
There was a problem hiding this comment.
nit: I’d note that the linter is usually run as part of make test.
There was a problem hiding this comment.
@addaleax Agree, but if the tests are throwing errors that will disable you to properly lint the code.
There was a problem hiding this comment.
@diosney Yep, I’ve seen the thread that prompted this PR, and I’m in favour of having the information of how to run only the linter noted here – I’d just want to save folks the trouble of thinking they have to run the linter separately under normal circumstances.
I don’t feel too strongly about it, and if you want to keep it this way, I won’t be in the way of that. :)
There was a problem hiding this comment.
@addaleax Oy! I think I understand you now! I just though you didn't see this addition as useful. Sorry for that.
I will add your proposal right now.
There was a problem hiding this comment.
@addaleax Just added your proposal, can you check it? Thanks!
|
LGTM, thanks! |
|
LGTM |
|
LGTM. A few nits that can be ignored or dealt with:
|
|
[x] Removed: I don't think linting is relevant to the BUILDING.md doc. So that change seems unnecessary. [x] Changed: [x] Added: Maybe mention that the linter won't run if any tests failed, since that was the particular gotcha that resulted in this PR. Ideally, there shouldn't be any failed tests, but if there are and you want to run the linter separately, here's how to do it... Can you review the new changes? |
|
LGTM |
|
Sorry, reworded line. Tell me if is OK now. |
|
LGTM as is. Final nit (which as always, you can ignore): I would go with wording like this:
This fixes a few very minor things (e.g., consistent verb tense). However, if there are reasons to prefer the current wording, that's fine. |
|
@Trott NP, changed to fix your nit. |
|
I know I said "final nit" before and I know this little bit is text I wrote, but I forgot that we try to avoid personal pronouns. So post-final nit: Get rid of
to
Once again, with or without this change, LGTM! |
|
LGTM |
|
Landed in fa46e50, thanks! |
Added clarification about the linter execution. PR-URL: #7534 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Added clarification about the linter execution. PR-URL: #7534 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Added clarification about the linter execution. PR-URL: #7534 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Added clarification about the linter execution. PR-URL: #7534 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Added clarification about the linter execution. PR-URL: #7534 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Added clarification about the linter execution. PR-URL: #7534 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Added clarification about the linter execution. PR-URL: #7534 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Added clarification about the linter execution. PR-URL: #7534 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Checklist
Affected core subsystem(s)
doc
Description of change
Right now there is no information in the building & testing docs on how to run the linter directly, which is useful so it can save time and separate tasks if one is checking only code style instead of checking the whole test suite.