[DOCS][INFRA] Use GitHub Actions problem matcher to simplify Unidoc log filtering#57015
[DOCS][INFRA] Use GitHub Actions problem matcher to simplify Unidoc log filtering#57015nchammas wants to merge 49 commits into
Conversation
the main build_and_test has too many custom bits that prevent problem matchers from working
|
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? |
|
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. |
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. |
|
I think it's useful, but I'm not sure how it will affect Spark's GA budget... cc @HyukjinKwon @zhengruifeng @gaogaotiantian |
|
I like this too but can we integrate this to our current building so it doesn't trigger another doc build? |
My goal is to migrate some checks out of the massive
I tried for several days to do this unsuccessfully. spark/.github/workflows/build_and_test.yml Lines 493 to 506 in ef58ab3 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 We do this so that we know PRs are always tested against the latest
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. |
|
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. |
|
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)). |
that's for another PR

What changes were proposed in this pull request?
-verboseoption fromJavaUnidoc / 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.
-verbosewas 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
-verbosegone, 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:
Was this patch authored or co-authored using generative AI tooling?
Co-authored with assistance from GitHub Copilot.