Simplify include analysis by enforcing the developer guide's include syntax #13230

practicalswift · 2018-05-14T08:00:44Z

When analysing includes in the project it is often assumed that the preferred bracket include syntax (#include <foo.h>) mentioned in developer-docs.md is used consistently. @sipa:s excellent circular dependencies script circular-dependencies.py (#13228) is an example of a script making this reasonable assumption.

This PR enables automatic Travis checking of the include syntax making sure that the bracket syntax includes (#include <foo.h>) is used consistently.

Empact · 2018-05-14T08:24:26Z

contrib/devtools/lint-includes.sh

leading / missing

Thanks! Fixed!

maflcko · 2018-05-19T16:56:48Z

src/test/blockchain_tests.cpp

Including a cpp file seems extremely wrong. (I know this is not your fault, just leaving a note)

sipa · 2018-05-19T19:09:34Z

The developer guidelines say to use the #include <...> form when possible. It's not clear to me when it wouldn't be possible.

If it's always possible, ACK on enforcing it.

promag · 2018-05-20T13:47:16Z

☝️then update developer notes?

practicalswift · 2018-05-21T07:17:35Z

@sipa @promag Good point! Updated the developer guidelines :-)

maflcko · 2018-05-29T17:50:23Z

Needs rebase

practicalswift · 2018-05-29T22:00:37Z

Thanks @MarcoFalke. Rebased!

Empact · 2018-06-01T13:06:57Z

utACK 04016bf could squash

maflcko · 2018-06-02T14:29:04Z

utACK 04016bf0b78aed2a6c39dfa284478b39192e2ec2

practicalswift · 2018-06-05T14:59:27Z

Rebased! Please re-review :-)

ken2812221 · 2018-06-05T15:10:35Z

test/lint/lint-includes.sh

Why not use *.cpp and *.h? Isn't it compatible with other git versions?

Yes, you're right that -- "*.cpp" "*.h" would be equivalent in this specific case (since we're only matching in src/), but my suggestions is that we keep this PR as is since it already has two utACK:s that would need re-reviewing in case of a change. Makes sense? :-)

Fixed when rebasing :-)

maflcko · 2018-06-05T15:14:32Z

utACK 186b0101854824bb2bed55a7674aa5d6979d9f66

ken2812221 · 2018-06-05T15:25:28Z

utACK 186b010

laanwj · 2018-06-05T18:06:32Z

src/bench/merkle_root.cpp

This doesn't compile locally here. Should be #include <bench/bench.h>

maflcko · 2018-06-05T22:31:17Z

Needs rebase

practicalswift · 2018-06-06T05:56:31Z

Rebased! Please re-review :-)

ken2812221 · 2018-06-06T06:15:22Z

src/test/blockchain_tests.cpp

This seems incorrect

In what way? :-)

It includes blockchain.h previously, but you include blockchain.cpp. Might because of #13288 merged.

Ouch, now fixed! Thanks for catching this. I missed the .cpp extension!

ken2812221

utACK 16e3cd3

Empact · 2018-06-07T00:04:12Z

re-utACK 16e3cd3

promag · 2018-06-07T01:46:30Z

utACK 16e3cd3.

maflcko · 2018-06-07T04:02:45Z

utACK 16e3cd3

@sipa

…ide's include syntax 16e3cd3 Clarify include recommendation (practicalswift) 6d10f43 Enforce the use of bracket syntax includes ("#include <foo.h>") (practicalswift) 906bee8 Use bracket syntax includes ("#include <foo.h>") (practicalswift) Pull request description: When analysing includes in the project it is often assumed that the preferred bracket include syntax (`#include <foo.h>`) mentioned in `developer-docs.md` is used consistently. @sipa:s excellent circular dependencies script [`circular-dependencies.py`](https://github.com/sipa/bitcoin/blob/50c69b78011c1bc55885ebfd216db60ed490ebea/contrib/devtools/circular-dependencies.py) (#13228) is an example of a script making this reasonable assumption. This PR enables automatic Travis checking of the include syntax making sure that the bracket syntax includes (`#include <foo.h>`) is used consistently. Tree-SHA512: a414921aabe8e487ebed42f3f1cbd02fecd1add385065c1f2244cd602c31889e61fea5a801507ec501ef9bd309b05d3c999f915cec1c2b44f085bb0d2835c182

@sipa

…oper guide's include syntax 16e3cd3 Clarify include recommendation (practicalswift) 6d10f43 Enforce the use of bracket syntax includes ("#include <foo.h>") (practicalswift) 906bee8 Use bracket syntax includes ("#include <foo.h>") (practicalswift) Pull request description: When analysing includes in the project it is often assumed that the preferred bracket include syntax (`#include <foo.h>`) mentioned in `developer-docs.md` is used consistently. @sipa:s excellent circular dependencies script [`circular-dependencies.py`](https://github.com/sipa/bitcoin/blob/50c69b78011c1bc55885ebfd216db60ed490ebea/contrib/devtools/circular-dependencies.py) (bitcoin#13228) is an example of a script making this reasonable assumption. This PR enables automatic Travis checking of the include syntax making sure that the bracket syntax includes (`#include <foo.h>`) is used consistently. Tree-SHA512: a414921aabe8e487ebed42f3f1cbd02fecd1add385065c1f2244cd602c31889e61fea5a801507ec501ef9bd309b05d3c999f915cec1c2b44f085bb0d2835c182 Signed-off-by: pasta <[email protected]>

ad5717d Lint: Add lint-includes.sh (Fuzzbawls) faba3f6 [tests] Use FastRandomContext in scheduler_tests.cpp (practicalswift) 2034bf6 Remove unused boost includes in reverselock_tests.cpp (Fuzzbawls) 3256d16 bench: Use non-throwing ParseDouble() (Fuzzbawls) 3c984d1 Remove duplicate includes (Fuzzbawls) b54e396 Declare TorReply parsing functions in torcontrol_tests (Fuzzbawls) Pull request description: This brings in the `lint-includes.sh` shell script from upstream (first introduced in bitcoin#11878) to automatically check for duplicate includes and expanded in bitcoin#13301 and bitcoin#13385 to check for the inclusion of `.cpp` files and the introduction of new boost includes, respectively. The check for enforcing bracket include syntax (bitcoin#13230) is also included, but currently disabled, as we have yet to systematically switch to that syntax preference. Three other upstream PRs are backported here as they are directly related to the removal of some boost dependencies and are very straight forward: - bitcoin#10547 - bitcoin#13291 - bitcoin#13383 **NOTE: #2711 removes the dependency on `boost/tuple/tuple.hpp`, so it makes sense to merge that first. submitting this now so the general concept/functionality can be reviewed prior to that PR being merged** ACKs for top commit: furszy: good ACK ad5717d random-zebra: utACK ad5717d and merging... Tree-SHA512: d9d32d6d5122dac52c9601cda75612d87c4e027c6f196a2382206b227fcfd2bb61d4f72df7cbf5e572d94150ad8ca6db6301bd99b0da647b9627fe342b66873f

@sipa

…oper guide's include syntax 16e3cd3 Clarify include recommendation (practicalswift) 6d10f43 Enforce the use of bracket syntax includes ("#include <foo.h>") (practicalswift) 906bee8 Use bracket syntax includes ("#include <foo.h>") (practicalswift) Pull request description: When analysing includes in the project it is often assumed that the preferred bracket include syntax (`#include <foo.h>`) mentioned in `developer-docs.md` is used consistently. @sipa:s excellent circular dependencies script [`circular-dependencies.py`](https://github.com/sipa/bitcoin/blob/50c69b78011c1bc55885ebfd216db60ed490ebea/contrib/devtools/circular-dependencies.py) (bitcoin#13228) is an example of a script making this reasonable assumption. This PR enables automatic Travis checking of the include syntax making sure that the bracket syntax includes (`#include <foo.h>`) is used consistently. Tree-SHA512: a414921aabe8e487ebed42f3f1cbd02fecd1add385065c1f2244cd602c31889e61fea5a801507ec501ef9bd309b05d3c999f915cec1c2b44f085bb0d2835c182 Signed-off-by: pasta <[email protected]>

practicalswift force-pushed the bracket-syntax-includes branch from 0e57f8d to 9d8f70a Compare May 14, 2018 08:05

Empact reviewed May 14, 2018

View reviewed changes

practicalswift force-pushed the bracket-syntax-includes branch from 9d8f70a to 1bf1635 Compare May 14, 2018 08:37

fanquake added the Scripts and tools label May 14, 2018

practicalswift changed the title ~~Allow for easy include analysis by enforcing the developer guide's include syntax preference~~ Simplify include analysis by enforcing the developer guide's include syntax May 14, 2018

practicalswift force-pushed the bracket-syntax-includes branch from 1bf1635 to 1187503 Compare May 15, 2018 15:11

maflcko reviewed May 19, 2018

View reviewed changes

Empact mentioned this pull request May 20, 2018

rpc: Remove the need to include rpc/blockchain.cpp in order to put GetDifficulty under test #13288

Merged

practicalswift force-pushed the bracket-syntax-includes branch from ed32311 to 5441186 Compare May 22, 2018 08:58

Empact mentioned this pull request May 23, 2018

lint: Add linter to error on #include <*.cpp> #13301

Merged

practicalswift force-pushed the bracket-syntax-includes branch from 5441186 to 04016bf Compare May 29, 2018 21:52

DrahtBot mentioned this pull request Jun 4, 2018

build: Guard against accidental introduction of new Boost dependencies #13385

Merged

practicalswift force-pushed the bracket-syntax-includes branch from 04016bf to 186b010 Compare June 5, 2018 14:58

ken2812221 reviewed Jun 5, 2018

View reviewed changes

laanwj reviewed Jun 5, 2018

View reviewed changes

maflcko added the Needs rebase label Jun 6, 2018

practicalswift force-pushed the bracket-syntax-includes branch from 186b010 to a3ddab8 Compare June 6, 2018 05:56

practicalswift force-pushed the bracket-syntax-includes branch from a3ddab8 to 655d432 Compare June 6, 2018 05:57

ken2812221 reviewed Jun 6, 2018

View reviewed changes

practicalswift added 3 commits June 6, 2018 11:09

Use bracket syntax includes ("#include <foo.h>")

906bee8

Enforce the use of bracket syntax includes ("#include <foo.h>")

6d10f43

Clarify include recommendation

16e3cd3

practicalswift force-pushed the bracket-syntax-includes branch from 655d432 to 16e3cd3 Compare June 6, 2018 09:09

ken2812221 approved these changes Jun 6, 2018

View reviewed changes

maflcko removed the Needs rebase label Jun 6, 2018

laanwj merged commit 16e3cd3 into bitcoin:master Jun 11, 2018

Bushstar mentioned this pull request Jun 12, 2018

commits from bitcoin/master FeatherCoin/Feathercoin#343

Merged

practicalswift deleted the bracket-syntax-includes branch April 10, 2021 19:34

Fuzzbawls mentioned this pull request Feb 7, 2022

[Lint] Introduce lint-includes.sh script PIVX-Project/PIVX#2740

Merged

bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022

Simplify include analysis by enforcing the developer guide's include syntax #13230

Simplify include analysis by enforcing the developer guide's include syntax #13230

Uh oh!

Conversation

practicalswift commented May 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sipa commented May 19, 2018

Uh oh!

promag commented May 20, 2018

Uh oh!

practicalswift commented May 21, 2018

Uh oh!

maflcko commented May 29, 2018

Uh oh!

practicalswift commented May 29, 2018

Uh oh!

Empact commented Jun 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Jun 2, 2018

Uh oh!

practicalswift commented Jun 5, 2018

Uh oh!

ken2812221 Jun 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

practicalswift Jun 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko commented Jun 5, 2018

Uh oh!

ken2812221 commented Jun 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko commented Jun 5, 2018

Uh oh!

practicalswift commented Jun 6, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ken2812221 Jun 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ken2812221 left a comment

Choose a reason for hiding this comment

Uh oh!

Empact commented Jun 7, 2018

Uh oh!

promag commented Jun 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Jun 7, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

practicalswift commented May 14, 2018 •

edited

Loading

Empact commented Jun 1, 2018 •

edited

Loading

ken2812221 Jun 5, 2018 •

edited

Loading

practicalswift Jun 5, 2018 •

edited

Loading

ken2812221 Jun 6, 2018 •

edited

Loading

promag commented Jun 7, 2018 •

edited

Loading