chore: add fix_includes.py for include cleanup workflow by lucasfang · Pull Request #25 · apache/paimon-cpp · GitHub
Skip to content

chore: add fix_includes.py for include cleanup workflow - #25

Merged
leaves12138 merged 2 commits into
apache:mainfrom
lucasfang:fix
May 29, 2026
Merged

chore: add fix_includes.py for include cleanup workflow#25
leaves12138 merged 2 commits into
apache:mainfrom
lucasfang:fix

Conversation

@lucasfang

Copy link
Copy Markdown
Collaborator

Purpose

Linked issue: close #TODO

Introduce build_support/fix_includes.py to support include cleanup and ordering workflow in build tooling.
This change adds one utility script and does not modify runtime code paths.

Tests

  1. python3 -m py_compile build_support/fix_includes.py (pass)
  2. Verified migrated file line count: 2498
  3. Confirmed file is tracked as a single new addition in this change set

API and Format

No impact on public API under include/ and no storage/protocol format change.

Documentation

No end-user feature introduced. This is a build/developer tooling migration.

Generative AI tooling

Generated-by: GitHub Copilot (GPT-5.3-Codex)

Copilot AI review requested due to automatic review settings May 27, 2026 07:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@leaves12138 leaves12138 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the include-cleanup helper. I found one workflow blocker compared with the upstream IWYU script behavior.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This currently always returns 0, even in --dry_run mode when files would be modified. The upstream fix_includes.py returns min(process_ret, 100) for dry runs, and the help text above also documents that behavior. With this change, CI/pre-commit workflows cannot use fix_includes.py --dry_run to fail when includes need cleanup, so include drift would be silently accepted. Please keep the upstream dry-run exit-code behavior (or update the workflow/docs if a different contract is intended).

@leaves12138 leaves12138 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged that this file is intended to remain a verbatim IWYU copy. Please disregard my previous dry-run exit-code comment as a blocker; preserving the upstream script unchanged is reasonable. I have no further blocking comments.

@leaves12138 leaves12138 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the latest update. The LICENSE now lists build_support/fix_includes.py under the include-what-you-use / UIUC-NCSA notice, and preserving the imported script unchanged is fine. I have no further comments.

@leaves12138
leaves12138 merged commit 478887b into apache:main May 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants