Add Copilot instructions for PR review - #7
Merged
Conversation
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.
Why
This repository had no
copilot-instructions.md, so Copilot reviewed its PRs with no idea whatthis code is for or what goes wrong in it.
The file added here is written for PR review, not onboarding — the five existing files in the
ecosystem were generated by Copilot's repository-onboarding feature in November 2025, and are
being rewritten in the same way (cms-flaf/FLAF#310 and its companions): what a useful comment
looks like here, the invariants worth checking with the failure signature each produces, the rule
that documentation ships in the same PR, and — deliberately — what not to comment on, since
noise is an automated reviewer's main cost.
Framework-wide rules are not duplicated; the file points at
FLAF/.github/copilot-instructions.mdfor them.
Testing
Documentation only — no code change. Every path, filename and directory referenced was verified to
exist in the current tree, and the facts table is dated so the next reader knows how much to trust
it.
What it says
PlotKit is the one repository in the ecosystem that is an ordinary installable Python package with
unit tests that run anywhere — no CVMFS, no grid proxy, no CMSSW. So "we cannot test this"
does not apply, and asking for a test alongside a change to
spec.py,config.py,histogram.py,rootcompat.pyor a plotter is a legitimate review comment. That is statedexplicitly.
The rest is what a screenshot would not reveal:
produces plots that differ by backend, silently, because both succeed.
changing a default breaks three analyses; silent default changes surface months later in a plot
that looks subtly different.
rootcompat.pyexists so the package works without it; a module-scopeROOT import breaks the pure-matplotlib install.
propagation, normalisation applied twice, a stack ordered by dictionary iteration.