Skip to content

[DOCS][INFRA] Use GitHub Actions problem matcher to simplify Unidoc log filtering#57015

Draft
nchammas wants to merge 49 commits into
apache:masterfrom
nchammas:unidoc-cleanup
Draft

[DOCS][INFRA] Use GitHub Actions problem matcher to simplify Unidoc log filtering#57015
nchammas wants to merge 49 commits into
apache:masterfrom
nchammas:unidoc-cleanup

Conversation

@nchammas

@nchammas nchammas commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  1. Use a GitHub Actions problem matcher to simplify how we surface doc errors on PRs and eliminate the need for custom logic in our documentation build script for this purpose.
  1. Remove the -verbose option from JavaUnidoc / unidoc / javacOptions, which is no longer needed.

Why are the changes needed?

In general, we should strive to minimize the amount of Ruby code we maintain since it's not a core competency of the project. In this particular case, I believe we can specifically use declarative GitHub Actions problem matchers to accomplish our goal without needing to write or maintain custom log filtering logic.

-verbose was added in #55605 and #55814 to surface doclint errors, along with filters to cut down the unwanted extra noise. However, the same PRs also added -Xmaxerrs 0, which independently fixes the same problem of surfacing doc errors, regardless of -verbose.

With -verbose gone, the log drops from ~16K lines to ~1K, making the remaining noise a minor nuisance rather than worth a complex filter.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

This PR and its associated CI runs will serve as tests.

TODO:

  • Validate that the problem matchers surface the desired log entries.

Was this patch authored or co-authored using generative AI tooling?

Co-authored with assistance from GitHub Copilot.

@nchammas

nchammas commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

I'm waist deep in some workflow experiments to try to simplify our PR builds and get PR line-level annotations to show up correctly.

I need a maintainer to approve the new workflow I just added here and, if possible, auto-approve new workflows so I can continue to work without repeatedly asking for manual approvals.

@gaogaotiantian or @cloud-fan (or any maintainer, really), could you please help?

@cloud-fan
cloud-fan marked this pull request as ready for review July 8, 2026 16:23
@cloud-fan

Copy link
Copy Markdown
Contributor

I don't think there is auto-approve, and we have been pushed hard to reduce Github Action usages. You can test it out in your Spark fork, which needs no approval.

@nchammas

nchammas commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

You can test it out in your Spark fork, which needs no approval.

I don't think that will help me since I'm specifically trying to get annotations to write to PRs on this repo. In any case, thank you for approving the workflow.

Inline annotations are working! This is very exciting.

@cloud-fan - I think if we updated several of our workflows to follow the pattern of this new docs workflow it would be a very nice productivity boost.

In addition to having annotations at the top of the workflow log, contributors would also have inline annotations right on the PR diff. They won't even have to open the workflow logs to see what's wrong.

Do you like the direction this is going in? I can wrap up this PR with the focus on javadoc errors and establishing this new pattern. In follow ups I can then migrate the various linter and other workflows to adopt this pattern and also place inline annotations.

@nchammas
nchammas marked this pull request as draft July 8, 2026 19:03
@cloud-fan

Copy link
Copy Markdown
Contributor

I think it's useful, but I'm not sure how it will affect Spark's GA budget... cc @HyukjinKwon @zhengruifeng @gaogaotiantian

@HyukjinKwon

Copy link
Copy Markdown
Member

I like this too but can we integrate this to our current building so it doesn't trigger another doc build?

@nchammas

nchammas commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

I'm not sure how it will affect Spark's GA budget

My goal is to migrate some checks out of the massive build_and_test.yml and into smaller workflows like docs.yml, so the impact should be neutral.

can we integrate this to our current building so it doesn't trigger another doc build?

I tried for several days to do this unsuccessfully. build_and_test.yml implements a non-standard checkout pattern that totally breaks GitHub's problem matchers.

- name: Checkout Spark repository
uses: actions/checkout@v6
# In order to fetch changed files
with:
fetch-depth: 0
repository: apache/spark
ref: ${{ needs.precondition.outputs.head_sha }}
- name: Sync the current branch with the latest in Apache Spark
if: github.repository != 'apache/spark'
run: |
echo "APACHE_SPARK_REF=$(git rev-parse HEAD)" >> $GITHUB_ENV
git fetch https://github.com/$GITHUB_REPOSITORY.git ${GITHUB_REF#refs/heads/}
git -c user.name='Apache Spark Test Account' -c user.email='sparktestacc@gmail.com' merge --no-commit --progress --squash FETCH_HEAD
git -c user.name='Apache Spark Test Account' -c user.email='sparktestacc@gmail.com' commit -m "Merged commit" --allow-empty

For problem matchers (and likely other productivity features of GitHub) to work correctly, they need the local checkout to have a reference to the same commit as what's on the PR. Because we checkout master and merge the PR into it, we create a new, synthetic commit that doesn't exist anywhere else.

We do this so that we know PRs are always tested against the latest master. That way tests don't pass on the PR but later fail post-merge because master changed. But we can address this concern in some other ways:

  1. Conservative approach with weaker guarantees than we currently have: Checkout the fork "normally" as I am doing in docs.yml. If the common root of the PR branch vs. target branch is stale (e.g. older than ~1 week), fail fast.
  2. Radical approach with the same guarantees as we currently have: Adopt a merge queue and overhaul dev/merge_spark_pr.py to use it.

If we are not willing to consider either approach, then I think it will be impossible to get inline PR annotations working. Over time, we may also find there are other GitHub features that are difficult or impossible to use because of our non-standard checkout pattern.

@nchammas

nchammas commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

An additional benefit of using standard fork checkouts, of course, is that we get a stable commit to test against since we are not repeatedly merging against a moving target. So any CI logic to capture and communicate a stable commit to the various workflow steps, like #55879, can just go away.

@nchammas

Copy link
Copy Markdown
Contributor Author

Noting here for the record: Another difficulty introduced by our checkout pattern is that it makes it harder to adopt composite actions (e.g. see #36924 (comment)).

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.

4 participants