Have the cherry-pick script do every relevant milestone #18975
Conversation
thejcannon
added
the
category:internal
|
Automate all the things! |
| fi | ||
| fi | ||
|
|
||
| # NB: Find all milestones >= $TARGET_MILESTONE by having GH list them, then uses awk to trim the |
There was a problem hiding this comment.
For the record, this much was clear all along. what wasn't clear was why that awk incantation achieves this. Assume that the reader understands piping and stuff, but doesn't know awk, because who does?
There was a problem hiding this comment.
Like 80% of any bash script is unintelligible symbols for me. I don't think we should be documenting how each line works. 🤷♂️
And it's kinda over-cliched at this point, but if you ask ChatGPT what it does it actually gives a nice summary 😉
There was a problem hiding this comment.
We expect people to have some basic bash familiarity, but awk is very rarely used and is especially unintelligible. And if we're relying on some non-obvious bashisms then I would expect to document those too.
Plus, if ChatGPT generated this then that makes me more nervous, not less...
What is the argument against more documentation here? "You can always go futz with ChatGPT or RTFM" doesn't seem like the most reader-friendly approach?
There was a problem hiding this comment.
It just feels a bit cherry-picked (pun intended). We aren't explaining graphQL, or the while IFS= read -r REVIEWER; do PR_CREATE_CMD+=(--reviewer "$REVIEWER"); done <<< "$REVIEWERS" line (which I don't understand, but meh I get the intent and this is a script)
| nodes {title} | ||
| } | ||
| } | ||
| }' --jq .data.repository.milestones.nodes.[].title | awk "/$TARGET_MILESTONE/{p=1}p" -) |
There was a problem hiding this comment.
So here's what's unclear: Why are we using $TARGET_MILESTONE as a regex? It is intended to be a literal string. And why are we searching for it as the pattern? And what does the action do? That apparent magic is what I think needs documenting.
There was a problem hiding this comment.
Ah ok that's much more explicit if a concern
There was a problem hiding this comment.
From my testing, this appears to rely on the fact that the milestones are returned in increasing order, so that must be verified as a property of the API (or if not, we must sort), and must be documented.
There was a problem hiding this comment.
They're in that order because we create them in that order. I can't imagine we'd be making milestones for later branches before earlier ones
There was a problem hiding this comment.
And just generally explaining the awk program would be really helpful.
There was a problem hiding this comment.
And yet if we do, this will fail in a very unpredictable and undocumented way. And sort is free, as is documenting why.
There was a problem hiding this comment.
sort isn't correct though, right?
$ gh api graphql -F owner=":owner" -F name=":repo" -f query='
> query ListMilestones($name: String!, $owner: String!) {
> repository(owner: $owner, name: $name) {
> milestones(last: 10) {
> nodes {title}
> }
> }
> }' --jq .data.repository.milestones.nodes.[].title | sort
2.10.x
2.11.x
2.12.x
2.13.x
2.14.x
2.15.x
2.16.x
2.17.x
2.8.x
2.9.x
There was a problem hiding this comment.
Oh hot damn there's a version sort?!?!?!?!
-V, --version-sort natural sort of (version) numbers within text
…8975) Now it will make a cherry to the target milestone and every milestone after it (defined by the order of milestone creation). The next step is to have this be run in a GitHub Action 🥳
Now it will make a cherry to the target milestone and every milestone after it (defined by the order of milestone creation).
The next step is to have this be run in a GitHub Action 🥳