Skip to content
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

Include Python.h first #59929

Merged
merged 1 commit into from
Oct 2, 2024
Merged

Include Python.h first #59929

merged 1 commit into from
Oct 2, 2024

Conversation

cdce8p
Copy link
Contributor

@cdce8p cdce8p commented Oct 1, 2024

Followup from #59906 (comment)
/CC @WillAyd

Move # include <Python.h> to a separate include block to force clang-format to keep the order and it as first include.

@cdce8p cdce8p requested a review from WillAyd as a code owner October 1, 2024 12:02
@WillAyd
Copy link
Member

WillAyd commented Oct 1, 2024

Looks good. Is there a clang-format argument we can pass or set to prevent the reorder?

@cdce8p
Copy link
Contributor Author

cdce8p commented Oct 1, 2024

Looks good. Is there a clang-format argument we can pass or set to prevent the reorder?

There is SortIncludes: Never. I'm just not sure that's really necessary. Include sorting in itself is probably desirable. It's just the preference to put the Python.h includes at the top. As long as they are in a separate include block, that shouldn't be an issue.

@mroeschke mroeschke added Build Library building on various platforms Refactor Internal refactoring of code labels Oct 1, 2024
@WillAyd WillAyd added this to the 3.0 milestone Oct 2, 2024
@WillAyd WillAyd merged commit fd823d2 into pandas-dev:main Oct 2, 2024
101 of 127 checks passed
@WillAyd
Copy link
Member

WillAyd commented Oct 2, 2024

Great thanks @cdce8p

@cdce8p cdce8p deleted the include-sorting branch October 2, 2024 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Library building on various platforms Refactor Internal refactoring of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants