-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
benchmark,doc,lib,test: capitalize comments #26223
Conversation
This updates a lot of comments.
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.
RSLGTM, but are you willing to backport to the four current release branches (11, 10, 8, 6) so as to avoid endless merge conflicts going forward?
(I guess if churn is a concern, we can always try 45 or 49 instead of 40?)
@Trott yes, I am willing to do that. So far it seemed like the introduction of the rule itself did not cause any pain in backporting. It's probably because it's mostly just a single line in a comment and this can mostly be resolved automatically. |
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.
With in-passing nits, feel free to ignore or to address in a fixup commit/other PR.
Co-Authored-By: BridgeAR <[email protected]>
Co-Authored-By: BridgeAR <[email protected]>
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.
Thanks @BridgeAR 👍 👍
This updates a lot of comments. PR-URL: nodejs#26223 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Anto Aravinth <[email protected]>
Landed in 9edce1e |
This needs a manual backport for v11.x, which should probably happen soon, given that this changed 200 files. It might also make sense to not land the bits that change the linter rules for the release branches, because that can get in the way of backporting this? (Maybe it could have been a separate commit? I know other people do it that way when modifying/adding/enabling linter rules.) |
This updates a lot of comments. PR-URL: #26223 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Anto Aravinth <[email protected]>
I backported this directly to the staging branch. @addaleax I'll separate the PRs in the future. That is indeed a good way of dealing with that. In this case I just ran |
This updates a lot of comments.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes