Repository navigation
ci: gate renames in Core Checks and docs that source checks read - #247
Merged
Merged
Conversation
Found in review of #233. Core Checks listed changed files with rename detection, while Full Validation and the Required Merge Gate's classifier pass --no-renames. A change that only moved src/foo.cpp to doc/user/ classified as public-docs in Core Checks, which skipped all its source jobs, and the merge gate accepted its green Core Checks Gate. The release-file detector of the release validator tests missed a move out of ci/release/ the same way, and the merge gate's jobs that confirm a lightweight profile listed files with rename detection too. All of them now pass --no-renames. The merge-gate contract test requires the three classifier inputs to use the same git commands, fails on any list of changed files in the three workflows that detects renames, and runs each list on a real rename for every event. doc/design/pq-transport.md quotes vectors that generate-pq-transport-vectors.py --check compares in the lint job, and feature_pq_quickstart.py runs the quick start of doc/user/network/pq-transport.md. Both classified as public-docs, so a change to only them ran neither check, and the next source change failed. They now classify as source, and a contract test fails if a document with either check's markers does not. lint-pq-doc-tokens.py skipped comments where pq is followed by a word character or a hyphen, so <!--pq-opt-->, <!--pqopt--> and <!--pq_field--> dropped their tokens silently. Every comment starting with pq now fails unless it is a token marker or a vectors block that the generator reads, matched with the generator's own pattern. Without binaries, the lint also accepted options of OptionsCategory::HIDDEN, such as -checkpoints, which -help-debug never lists, so a docs-only change could pass it and fail feature_pq_quickstart.py later. It now leaves them out, as -help does. The self-test covers the misspellings and a hidden option, and test/lint/README.md describes both rules. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
4 tasks done
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
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.
CI and lint fixes from review of #233.
--no-renameswhen listing changed files. Core Checks, its release-file detector and four of the Required Merge Gate's jobs did not.src/foo.cppintodoc/user/classified as public docs in Core Checks, which skipped every source job, and the merge gate accepted its green Core Checks Gate.ci/release/skipped the release validator tests the same way.--no-renames. The merge-gate contract test fails on any rename-detecting file listing in the three workflows, and runs every "Write changed file list" step on real renames for each event.generate-pq-transport-vectors.py --checkin the lint job, and the operator guide's quick start byfeature_pq_quickstart.py. Both docs classified as public docs, so a change to only them ran neither check, and the next source PR failed. They now classify as source, and a contract test fails if a document carrying either check's markers doesn't.lint-pq-doc-tokens.pyno longer skips misspelled markers.<!--pq-opt-->,<!--pqopt-->and<!--pq_field-->used to drop their tokens silently. Every comment starting with<!--pqnow fails unless it is a token marker, or a vectors block the generator reads (matched with the generator's own pattern).OptionsCategory::HIDDENoptions, which-help-debugdoesn't list. That matches whatfeature_pq_quickstart.pychecks.test/lint/README.mdis updated to match.Tests run, all passing:
test_classify_merge_profile.pytest_pq_merge_gate_contract.py(64 failures against the unfixed workflows)test_ci_pq_contract.py,test_merge_gate_poller.py,test_classifier_ci_contract.py,test_scheduled_validation_contract.pyandtest_ibd_timeout_wiring.pylint-pq-doc-tokens.py, including its self-testSince #245, pull requests into
feature/pq-transportdon't run CI. #233's full CI covers this change once it merges.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.