Skip to content

Conversation

mbutrovich
Copy link
Contributor

Which issue does this PR close?

Closes #2427.

Rationale for this change

We want to make sure configs.md is in sync with the code.

What changes are included in this PR?

CI check to diff the docs folder after building.

How are these changes tested?

N/A

Co-Authored-By: Claude [email protected]

  Ensure that PRs include necessary documentation updates when config
  defaults change. The check runs after Maven build (which generates
  docs from config values) and fails if any docs files are modified,
  indicating missing documentation changes.

  Fixes apache#2427

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <[email protected]>
@mbutrovich
Copy link
Contributor Author

I could test this by submitting a PR with a branch based on a commit before #2428

mbutrovich added a commit to mbutrovich/datafusion-comet that referenced this pull request Sep 21, 2025
@mbutrovich
Copy link
Contributor Author

Looking at #2433, I don't think it worked correctly.

@mbutrovich mbutrovich removed the request for review from andygrove September 21, 2025 15:56
@mbutrovich mbutrovich marked this pull request as draft September 21, 2025 15:56
echo "Please run 'make jvm' locally and commit the updated documentation files."
exit 1
else
echo "✅ Documentation is up-to-date"
Copy link
Contributor

@coderfender coderfender Sep 21, 2025

Choose a reason for hiding this comment

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

Super minor nitpick to probably remove the emojis

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'll clean it up. Not impressed by my first Claude PR considering it also doesn't seem to work.

mbutrovich added a commit to mbutrovich/datafusion-comet that referenced this pull request Sep 22, 2025
mbutrovich added a commit to mbutrovich/datafusion-comet that referenced this pull request Sep 22, 2025
echo "Checking documentation consistency..."
# Configure git for safe directory access in container
git config --global --add safe.directory "$(pwd)"
Copy link
Member

Choose a reason for hiding this comment

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

This seems overly complex. I think we can just run a git diff to see if there are local changes?

ChatGPT suggested:

 - name: Fail if there are uncommitted changes
        run: |
          if ! git diff --quiet || ! git diff --cached --quiet; then
            echo "❌ Found uncommitted changes after build/format"
            git status
            git diff
            exit 1
          fi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems overly complex.

That's Claude for you! I'll give your suggestion a shot, thanks!

@mbutrovich
Copy link
Contributor Author

Punting on this for now.

@mbutrovich mbutrovich closed this Oct 6, 2025
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.

CI should check if new docs need to be generated
3 participants