To reproduce changes:
0$ find /usr -type f -name run-clang-tidy.py
1/usr/share/clang/run-clang-tidy.py
0$ compiledb -n make
0$ cd src
1$ /usr/share/clang/run-clang-tidy.py -checks='-*,performance-*' -p .. -fix
Performance improvement of this change may be small, but anyway nice to have.
To reproduce changes:
1. Install, then find run-clang-tidy.py
$ find /usr -type f -name run-clang-tidy.py
/usr/share/clang/run-clang-tidy.py
2. You need to create compile_commands.json file.
Run autogen, configure, then
$ compiledb -n make
3. Run clang-tidy with checks:
$ cd src
$ /usr/share/clang/run-clang-tidy.py -checks='-*,performance-*' -p .. -fix
@Warchant First: welcome as a future contributor! :)
My guess is that a massive automated refactoring like this with many unrelated changes is unlikely to get merged.
If you want to proceed with this type of work I suggest picking a single clang-tidy fix-it rule that results in a change set that a.) makes sense (ideally: everyone should agree on that the fix-it version is better than status quo version), b.) is trivially correct (ideally: if-it-compiles-it-is-correct), c.) makes the code more in line with what the developer notes suggest, and d.) doesn’t touch too many lines.
Of the changes in your PR I guess the function signature transformation from T f(std::string s) to T f(const std::string& s) is scoring (relatively) highest given these criteria.
And generally: don’t touch dependencies (src/leveldb/ in this case) and try to avoid touching consensus critical code.
Hope that helps! :)
There’s nothing wrong with Travis, you’ve modified a subtree, and the linter is failing as expected. Subtree modifications should always be sent to the relevant upstream repo. See: https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#subtrees .
In general, PRs that just run some automated tooling against the repo are unlikely to be merged, especially if they come from new contributors, see: https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#refactoring.
Also, you’ve said this is a performance improvement, but haven’t provided any benchmarks.
On Fri, 7 Aug 2020 at 05:26, Bohdan notifications@github.com wrote:
Something is wrong with Travis
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/19675#issuecomment-670200235, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGS34TSAOMEFT7AY6UW7U3R7MNZLANCNFSM4PW7VTHQ .
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
