chore(github): rebuild the issue, discussion and automation setup - #9
Merged
Conversation
There was one form, for bugs, and no config.yml, so anything that was not a
crash arrived as free text with no route of its own. Issue 2 was a question
about using train and predict on another organism; it sat for weeks because
nothing distinguished it from a defect report.
Four forms now, each asking what its own triage actually needs:
bug_report how it was installed, the exact command, full output.
The install method alone separates a Bioconda problem
from a source build.
unexpected_results new, and the important one. A wrong call is worse than a
crash because nothing announces it. It asks how the
expected answer is known, which panel and which reference
genome, and for a -vv log, because a panel read against a
different assembly is the usual answer and it is silent.
feature_request asks for the analysis that is awkward, not the feature.
documentation separates wrong from missing, since wrong is worse.
config.yml routes questions to Q&A, points at the docs and the FAQ, sends
marker panels to their discussion category, and asks for vulnerabilities
privately. Blank issues stay available: a small tool gets questions that fit
no form, and having nowhere to put them is worse than an unstructured issue.
The old form suggested "e.g., 0.2.1" as a version while the project was on
1.0.1, which is the kind of detail that tells a reporter nobody is reading.
…y run
None of this worked, and nothing said so.
The discussion form was at DISCUSSION_TEMPLATE/marker_proposal.yml. GitHub
matches that filename against a discussion category slug, and the categories
here are announcements, general, ideas, polls, q-a and show-and-tell. There is
no marker_proposal category, so the file was ignored and no one was ever shown
the form. Renamed to marker-proposals.yml, which needs the matching category.
The workflow had never run at all, and carried three faults waiting for the day
it did:
* The label step fired only on titles containing "[Marker Proposal]", but the
form's prefix was "[Lineage Markers]". They could not both be right.
* Even on a match it called createLabel and stopped, so it created a label and
applied it to nothing. Labelling a discussion needs addLabelsToLabelable
through GraphQL, not the REST issues API.
* It rejected valid proposals. The allele check was
['A','C','G','T'].includes(ref), which is a single character, while the
format accepts multi-base REF and ALT for MNVs and indels. It would have
answered a correct panel with "invalid ref allele".
The validator now lives in .github/scripts/validate-marker-tsv.js with its own
tests, rather than buried in YAML where it could not be exercised. It follows
docs/marker_format.md: the lineage hierarchy runs from column 4 until the first
empty cell, whatever follows that cell is gene and mutation, alleles are
multi-base ACGT, and allele length is checked against both the k-mer window and
the classify flank limit. It also flags the mistake the documentation calls the
most common one, a gene column with no empty separator before it, where the
annotations are silently swallowed into the hierarchy.
Every test case is copied from the documentation, so the two cannot drift apart
without a test failing. The suite was checked by mutation rather than by
reading: reintroducing the original single-character allele bug fails 5 tests,
removing the empty-separator rule fails 2, and dropping the tab check fails 1.
The workflow now keys on the category slug alone, so no wording has to agree
with any other wording. It runs the validator's tests before it is allowed to
comment, and it edits its previous comment instead of stacking a new one under
every edit.
The relative link to ../docs/marker_format.md, which resolves to nothing from a
discussion, is now the documentation site.
Every GitHub link in this file pointed at github.com/mycolega/pathotypr, which returns 404. So "Open a Discussion", "Use Issues" and the feature request link, the three things the file exists to tell people, all led nowhere. The marker checklist also contradicted docs/marker_format.md. It gave the format as a flat "position ref alt lineage" and said single nucleotide variants only, no indels. The format is position, REF, ALT, then one lineage level per column until an empty separator, then gene and mutation; and equal-length MNVs work in both workflows while indels are accepted by classify and skipped by split-fastq. Someone following the checklist would have built a panel that does less than it could, or been told their valid panel was wrong. Adds a table at the top routing each kind of contribution to the form or the discussion category that now handles it, and points the format rules at the documentation site rather than restating them here, since restating them is how they came to disagree.
…olicy The pull request template asks for what a reviewer cannot reconstruct: what changed, why, and how it was verified. That last section asks for the command and its output rather than a claim that it should work, because the difference between those two is most of what review is for. SECURITY.md states where a vulnerability goes and, more usefully, what counts as one here. A parsing crash on a crafted FASTA is in scope; so is anything that gets a download past the URL validation or the SSRF guard in the desktop app, and so is a model bundle that does something other than load a model. A marker panel that misclassifies is not, and is routed to the form built for it. The code of conduct is the Contributor Covenant, with the enforcement contact filled in. The copy this was taken from has that line blank, so it asks people to report abuse to nobody.
Three pieces that work together. labeler.yml puts the module and area labels on a pull request from the files it changes, so "what is broken in the GUI" becomes a question the issue list can answer. Labels applied by hand are labels that stop being applied. It uses pull_request_target because a fork's pull request gets a read-only token otherwise, and it never checks out or runs the branch it is labelling: it reads filenames and writes labels. sync-labels is off so it cannot remove a label a person put there deliberately. The .github rule is a glob over the whole directory rather than a list, because a list would leave the file adding to it uncovered. dependabot.yml keeps cargo and the workflow actions current, monthly rather than weekly and grouped, with majors held apart in their own pull request. The schedule there is Dependabot's own timer, not a cron in Actions; no workflow in this repository runs on a schedule. dependabot-auto-merge.yml merges the routine group once CI is green and never a major. The rule is written positively, naming patch and minor, rather than asking whether an update is "not major": an ecosystem that reports no update type yields an empty string, an empty string is not equal to major, and the negated form would wave through something nobody identified. This also closes the gap that let an outside bot open the OpenSSL pull request before anything here noticed. GitHub security advisories against a dependency open their own pull request the day they land, independently of the monthly cadence.
Paururo
added
area:ci
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.
Brings the repository's community configuration up to the shape used in
get_MNV, and fixes the parts of it here that did not work.What did not work
The marker proposal form had never been shown to anyone. GitHub matches a file in
DISCUSSION_TEMPLATE/against a discussion category slug. The file wasmarker_proposal.yml; the categories areannouncements,general,ideas,polls,q-aandshow-and-tell. No match, so the file was ignored silently.validate-markers.ymlhad never run, and carried three faults waiting for the day it did:[Marker Proposal]; the form produced[Lineage Markers]createLabeland stopped, so it created a label and applied it to nothing. Labelling a discussion needsaddLabelsToLabelablethrough GraphQL['A','C','G','T'].includes(ref), a single character, while the format accepts multi-base REF and ALT. It would have answered a correct panel with "invalid ref allele"Every GitHub link in
CONTRIBUTING.mdpointed atgithub.com/mycolega/pathotypr, which returns 404. "Open a Discussion", "Use Issues" and the feature request link all led nowhere. Its marker checklist also contradicteddocs/marker_format.md, giving the format as a flatposition ref alt lineageand claiming SNPs only.Issue forms
There was one form, for bugs, and no
config.yml. Issue #2 was a question about usingtrainandpredicton another organism; it sat for weeks because nothing distinguished it from a defect report.-vvlogconfig.ymlroutes questions to Q&A, points at the docs and the FAQ, sends marker panels to their category, and asks for vulnerabilities privately. Blank issues stay available, as inget_MNV.The old form suggested
e.g., 0.2.1as a version while the project was on 1.0.1.The validator, rewritten and tested
It now lives in
.github/scripts/validate-marker-tsv.jswith its own tests, rather than buried in YAML where it could not be exercised. It followsdocs/marker_format.md: the lineage hierarchy runs from column 4 until the first empty cell, whatever follows is gene and mutation, alleles are multi-base ACGT, and allele length is checked against both the k-mer window and the classify flank limit.It also flags the mistake the documentation calls the most common one: a gene column with no empty separator before it, where the annotations are silently swallowed into the hierarchy.
Every test case is copied from the documentation, so the two cannot drift apart without a test failing.
The suite was checked by mutation rather than by reading. Reintroducing the original single-character allele bug fails 5 tests; removing the empty-separator rule fails 2; dropping the tab check fails 1.
The workflow keys on the category slug alone, so no wording has to agree with any other wording. It runs the validator's tests before it is allowed to comment, and it edits its previous comment instead of stacking a new one under every edit.
Automation
labeler.ymlapplies module and area labels from the files a pull request changes.pull_request_target, because a fork gets a read-only token otherwise; it reads filenames and writes labels and never checks out the branch.sync-labelsoff so it cannot remove a label a person put there.dependabot.ymlkeeps cargo and the workflow actions current, monthly and grouped, majors held apart. That schedule is Dependabot's own timer; no workflow here runs on a cron.dependabot-auto-merge.ymlmerges the routine group once CI is green, never a major. The rule names patch and minor rather than asking whether an update is "not major", because an ecosystem reporting no update type yields an empty string, and an empty string is not equal to major.Also
A pull request template, a security policy that says what counts as a vulnerability here and what does not, and the Contributor Covenant with the enforcement contact filled in.
Applied outside this branch
These are repository settings and cannot travel in a pull request:
mainprotected:testandGUI (Linux)required, pull request required with 0 approvals, force pushes and deletions off, admins not enforced so there is still an escape hatch.Still needed
The Marker proposals discussion category has to be created by hand: there is no API mutation for it.
Settings,Discussions,Categories,New category, named exactly Marker proposals so the slug ismarker-proposals. Until then the form stays dormant, exactly as its predecessor was.