src,crypto: adjust crypto_bio files for formatter/linter#42668
src,crypto: adjust crypto_bio files for formatter/linter#42668Trott wants to merge 1 commit intonodejs:masterfrom
Conversation
Run clang-format on the files and provide needed additional comments for the linter. Refs: nodejs#42665 (comment)
|
Review requested:
|
| if (avail > left) | ||
| avail = left; | ||
| if (avail > left) avail = left; |
There was a problem hiding this comment.
I find changes like this less readable and I'm guessing I'm not alone. (It further suggests to me that we don't actually use clang-format.) I imagine there's a configuration that could be changed in .clang-format if this is a highly-undesriable change (or we could add { and } which would presumably cause clang-format to leave the block on it's own line, although I haven't tested that).
|
Instead of introducing changes purely to satisfy the linters, what do you think about setting up the clang-format linter on CI first? It should automatically suggest fixes for areas touched by newer PRs. |
When you say "clang-format linter", do you mean something different than the |
Please don't let the "I had planned" part stop you or anyone else from doing this. I very well may never bother to do it. |
I meant
Sure, sent a PR to run it on CI - #42681. |
|
@RaisinTen In your opinion, should I mark this as blocked on #42681? Close this entirely? Reduce this to just the changes that the linter needs? Something else? |
|
IMO, we should close this PR because landing #42681 should automatically help in fixing the formatting in newer PRs and running clang-format on these files manually would potentially cause more git conflicts and add another git-blame to the logs without much gain. |
Run clang-format on the files and provide needed additional comments for
the linter.
Refs: #42665 (comment)