IGNITE-28821 Protected Classes CI check: report as warning instead of failing the PR build#13279
Open
anton-vinogradov wants to merge 2 commits into
Open
IGNITE-28821 Protected Classes CI check: report as warning instead of failing the PR build#13279anton-vinogradov wants to merge 2 commits into
anton-vinogradov wants to merge 2 commits into
Conversation
… failing the PR build Replace the hard `exit 1` with a separate non-blocking check "Rolling upgrade compatibility" (action_required conclusion), keeping the existing PR comment and `compatibility` label. The main "Rolling Upgrade check" job now always succeeds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
anton-vinogradov
added a commit
that referenced
this pull request
Jun 30, 2026
…of the detection script (#13280) ## What Simplify the detection logic in the `Rolling Upgrade check` step so it reads as a sequence of named steps: - Split it into small functions — `changed_java_files`, `is_protected_class`, `filter_protected` — with a top-level `changed_java_files | filter_protected > "$HITS_FILE"`. - Collapse the two near-identical loops (added/deleted vs modified files) into that single pass; the revision inspected per file status is `A` → head, `D`/`M` → base. - Drop the string accumulation with literal `\n` + `printf '%b'`, the process substitution and the parallel `HITS` variable; matches go straight to the output file, detected with `[ -s ]`. ## Why The two loops differed only in the diff filter and the grepped revision (duplication); the `\n`/`%b` and process-substitution idioms add reader friction. The named functions make the top level read as intent, halve the cyclomatic/cognitive complexity and remove the duplication. ## Behaviour Verified equivalent to the previous logic on add / modify / delete / rename scenarios, and that a PR with no hits still exits 0 under `set -eo pipefail`. One intentional difference: the content-based grep no longer flags add/delete of the annotation's own definition file `org/apache/ignite/internal/Order*.java` — the previous diff-based grep matched the file *path* in diff headers (the regex dots match slashes), a false positive, since that file is the `@Order` definition, not a protected class. ## Notes - Independent of [#13279](#13279) (different hunk of the same file); no conflict expected. Jira: https://issues.apache.org/jira/browse/IGNITE-28822 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The
Protected Classesworkflow no longer fails the PR build when a change touches@Order-annotated (protected) classes. Instead it publishes a separate, non-blocking checkRolling upgrade compatibilitywith theneutralconclusion, keeping the existing PR comment andcompatibilitylabel.Why
A hard failure (
exit 1) red-flagged PRs even for intentional, compatible changes, creating friction (the check was disabled manually on 2026-05-27 for this reason). A non-blocking advisory check surfaces the rolling-upgrade risk for reviewer attention without failing the build.Why
neutral(and notaction_required)GitHub's Checks API has no dedicated "warning" conclusion.
action_requiredis intended for GitHub Apps that attach a remediation action: it renders as a red ✕ with a "Resolve" button in the Checks tab and is dropped from the PR merge-box rollup entirely (so the warning becomes invisible there).neutralis the proper primitive for an advisory result — it shows in its own non-blocking "neutral" group in the merge box and as a grey marker in the Checks tab, and it never blocks merging.Notes
Rolling Upgrade checkjob now always succeeds.checks: writepermission added for the Checks API call.Jira: https://issues.apache.org/jira/browse/IGNITE-28821
🤖 Generated with Claude Code