Improve copilot instructions#3575
Merged
Merged
Conversation
Close some gaps that let the Copilot reviewer raise confident-but-wrong findings when the instructions are silent on a domain.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the repository’s Copilot reviewer guidance (.github/copilot-instructions.md) to reduce confident-but-incorrect review findings by documenting RSKj-specific release/versioning conventions, the canonical container build verification pattern, and a verify-before-asserting-breakage review approach.
Changes:
- Adds a Versioning and release tags section describing the repo’s tag/version conventions and how
version.propertiesmaps into artifact versions. - Adds a Containerized build and reproducible builds section documenting
/Dockerfileas the canonical container build and its establishedgpg --verify --output ... && sha256sum --check ...verification pattern. - Adds PR-review guidance bullets emphasizing observed evidence for breakage claims and holistic diff review, plus a scoping caveat under “Trust these instructions”.
|
|
||
| `/Dockerfile` is the canonical container build for the node and the reference for "how RSKj is built in a container". It bootstraps with `./configure.sh` and verifies that script against a signed checksum with `gpg --verify --output SHA256SUMS SHA256SUMS.asc && sha256sum --check SHA256SUMS` (`SHA256SUMS.asc` is a cleartext-signed file; `--output` extracts the payload and the `&&` chain gates the build on a good signature). This exact pattern is established and working — do **not** flag it as broken or claim the output file "is never created". | ||
|
|
||
| The workflow and templates under `.github/reproducible-build/` exist to **mirror** `/Dockerfile` for a published tag, so prefer consistency with `/Dockerfile` over alternative idioms; any change to the verify/build sequence should be made in `/Dockerfile` and the templates **together**, not in one alone. Before flagging a shell or Docker idiom here as incorrect, confirm it is not already the established, working pattern in `/Dockerfile`, `build_and_test.yml`, or `lint-java-code.yml`. |
Collaborator
Author
There was a problem hiding this comment.
Nice catch, but it will be present once this lands: #3573
|
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.
Adds targeted guidance to
.github/copilot-instructions.mdso the Copilot reviewer (and any agent that follows the file) stops producing confident-but-wrong findings on topics the file was previously silent about. Documentation/process only — no code changes. Four additions:Versioning and release tags(new section): documents that releases are taggedCODENAME-X.Y.Z, themodifierfield is that codename and is non-empty on every release tag, and that an emptymodifier/SNAPSHOTis a development/local-build state only. The existingfatJarempty-modifier note now points here.Containerized build and reproducible builds(new section): names/Dockerfileas the canonical container build and documents its establishedgpg --verify --output SHA256SUMS SHA256SUMS.asc && sha256sum --check SHA256SUMSverification ofconfigure.sh, so it isn't flagged as broken; notes the.github/reproducible-build/templates mirror it and should change together with/Dockerfile.PR review prioritiesbullets: "Ground claims of breakage in observation" and "Review the change holistically" (don't flag a branch an earlier guard already makes unreachable).Trust these instructions: for review correctness the "trust, don't explore" bias flips — a claim that existing code is defective must be backed by direct observation, not inferred from the document's silence.Motivation and Context
While reviewing #3573 (the reproducible-build CI workflow), the Copilot reviewer raised a cluster of confident but incorrect findings:
modifierand the workflow would reject valid tags / render9.0.3-— but the modifier is the network-upgrade codename and is always present on a release tag;gpg --verify --output …chain was broken and would never create the checksum file — but that is the exact pattern the repo's own/Dockerfileuses to build and ship every release.Each miss traced back to the instructions being silent on that domain, so the reviewer filled the gap from general training knowledge and asserted it confidently. This change encodes the missing domain facts (release-tag/modifier convention, the canonical container-build verification pattern) plus a verify-before-claiming principle, so the same gaps don't keep generating noise on future PRs.
How Has This Been Tested?
Documentation only — no build or runtime behaviour changes. The added facts were verified directly against the repository.
Types of changes
Checklist: