Sort includes #7326
Merged
Sort includes #7326
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #7326 +/- ##
==========================================
- Coverage 89.57% 89.05% -0.53%
==========================================
Files 278 278
Lines 59973 59974 +1
Branches 7469 7469
==========================================
- Hits 53723 53409 -314
- Misses 4101 4346 +245
- Partials 2149 2219 +70 |
CyberDem0n
reviewed
Nov 7, 2023
JelteF
previously requested changes
Nov 8, 2023
Initially I was thinking of not enforcing it on CI, especially since it was looking like a need to introduce a new tool. I'd like to keep the includes in a reasonable shape after we merge this. And as it turns out, it might actually find issues where includes are not always self contained, like the one causing the identification of PG16 RC1 being used by parts of our CI system. |
thanodnl
force-pushed
the
sort-includes
branch
2 times, most recently
from
d7a158a
to
6f54735
Compare
Yeah sounds, good let's add it to CI and probably also |
thanodnl
force-pushed
the
sort-includes
branch
from
6f54735
to
a3020b6
Compare
CyberDem0n
reviewed
Nov 14, 2023
thanodnl
force-pushed
the
sort-includes
branch
2 times, most recently
from
0d32a3d
to
b158d22
Compare
onurctirtir
approved these changes
Nov 23, 2023
thanodnl
force-pushed
the
sort-includes
branch
2 times, most recently
from
b055f51
to
88bb019
Compare
thanodnl
force-pushed
the
sort-includes
branch
from
88bb019
to
8900f46
Compare
thanodnl
dismissed
JelteF’s stale review
Addressed feedback. He is out of office and not able to re-review. Onur has reviewed instead
JelteF
pushed a commit
that referenced
this pull request
Apr 16, 2024
CHERRY-PICK NOTES: This cherry-pick only includes the scripts, not the actual changes. These are done in a follow up commit to ease further backporting. This change adds a script to programatically group all includes in a specific order. The script was used as a one time invocation to group and sort all includes throught our formatted code. The grouping is as follows: - System includes (eg. `#include<...>`) - Postgres.h (eg. `#include "postgres.h"`) - Toplevel imports from postgres, not contained in a directory (eg. `#include "miscadmin.h"`) - General postgres includes (eg . `#include "nodes/..."`) - Toplevel citus includes, not contained in a directory (eg. `#include "citus_verion.h"`) - Columnar includes (eg. `#include "columnar/..."`) - Distributed includes (eg. `#include "distributed/..."`) Because it is quite hard to understand the difference between toplevel citus includes and toplevel postgres includes it hardcodes the list of toplevel citus includes. In the same manner it assumes anything not prefixed with `columnar/` or `distributed/` as a postgres include. The sorting/grouping is enforced by CI. Since we do so with our own script there are not changes required in our uncrustify configuration. (cherry picked from commit 0620c8f)
JelteF
pushed a commit
that referenced
this pull request
Apr 16, 2024
CHERRY-PICK NOTES: This cherry-pick only includes the scripts, not the actual changes. These are done in a follow up commit to ease further backporting. This change adds a script to programatically group all includes in a specific order. The script was used as a one time invocation to group and sort all includes throught our formatted code. The grouping is as follows: - System includes (eg. `#include<...>`) - Postgres.h (eg. `#include "postgres.h"`) - Toplevel imports from postgres, not contained in a directory (eg. `#include "miscadmin.h"`) - General postgres includes (eg . `#include "nodes/..."`) - Toplevel citus includes, not contained in a directory (eg. `#include "citus_verion.h"`) - Columnar includes (eg. `#include "columnar/..."`) - Distributed includes (eg. `#include "distributed/..."`) Because it is quite hard to understand the difference between toplevel citus includes and toplevel postgres includes it hardcodes the list of toplevel citus includes. In the same manner it assumes anything not prefixed with `columnar/` or `distributed/` as a postgres include. The sorting/grouping is enforced by CI. Since we do so with our own script there are not changes required in our uncrustify configuration. (cherry picked from commit 0620c8f)
JelteF
pushed a commit
that referenced
this pull request
Apr 17, 2024
CHERRY-PICK NOTES: This cherry-pick only includes the scripts, not the actual changes. These are done in a follow up commit to ease further backporting. This change adds a script to programatically group all includes in a specific order. The script was used as a one time invocation to group and sort all includes throught our formatted code. The grouping is as follows: - System includes (eg. `#include<...>`) - Postgres.h (eg. `#include "postgres.h"`) - Toplevel imports from postgres, not contained in a directory (eg. `#include "miscadmin.h"`) - General postgres includes (eg . `#include "nodes/..."`) - Toplevel citus includes, not contained in a directory (eg. `#include "citus_verion.h"`) - Columnar includes (eg. `#include "columnar/..."`) - Distributed includes (eg. `#include "distributed/..."`) Because it is quite hard to understand the difference between toplevel citus includes and toplevel postgres includes it hardcodes the list of toplevel citus includes. In the same manner it assumes anything not prefixed with `columnar/` or `distributed/` as a postgres include. The sorting/grouping is enforced by CI. Since we do so with our own script there are not changes required in our uncrustify configuration. (cherry picked from commit 0620c8f)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change adds a script to programatically group all includes in a specific order. The script was used as a one time invocation to group and sort all includes throught our formatted code. The grouping is as follows:
#include<...>)#include "postgres.h")#include "miscadmin.h")#include "nodes/...")#include "citus_verion.h")#include "columnar/...")#include "distributed/...")Because it is quite hard to understand the difference between toplevel citus includes and toplevel postgres includes it hardcodes the list of toplevel citus includes. In the same manner it assumes anything not prefixed with
columnar/ordistributed/as a postgres include.The sorting/grouping is enforced by CI. Since we do so with our own script there are not changes required in our uncrustify configuration.