https://pantsbuild.org/ logo
#development
Title
# development
h

happy-kitchen-89482

07/07/2022, 11:48 PM
I'm going to restore the branch checks to just look for "Merge OK" since AFAIK the notion that this didn't work was in error. But @witty-crayon-22786 let me know if you think otherwise.
w

witty-crayon-22786

07/07/2022, 11:49 PM
it wasn’t working i don’t think. my guess is that it was looking at “the wrong one”, given that there are two
but yea, give it a shot.
h

happy-kitchen-89482

07/07/2022, 11:54 PM
It goes by the name string, so having two with the same name is the official way to do this (I got the idea from GHA docs)
But let's give it a go
w

witty-crayon-22786

07/07/2022, 11:54 PM
can go look at the example i linked in the previous thread. it auto-merged after the first Merge OK
b

bitter-ability-32190

07/08/2022, 12:10 AM
Yeah FWIW Benjy I can merge https://github.com/pantsbuild/pants/pull/15951 with `Merge OK skipped
And failing builds
It says its required, but its skipped. I think that counts as "passed"
h

happy-kitchen-89482

07/08/2022, 12:14 AM
Skipped should not count as passed, but the opposite
that's why we need to jump through hoops
Oh
Nuts
🥜 2
This is not true for conditionals, only for path filtering etc.
GodDAMN it GHA
why you gotta be so silly
So there is basically no solution to this
So skipped jobs sometimes count as passed and sometimes not, depending on why they were skipped
😬 2
Just great
I'll think on this
b

busy-vase-39202

07/08/2022, 12:28 AM
Yiiiiikes
b

bitter-ability-32190

07/08/2022, 12:32 AM
FWIW in those solutions, you can probably just use
gh
which is in the default container image for GHA runners
👀 1
(I should've been more explicit, using
gh
instead of the crazy
curl
command)
h

happy-kitchen-89482

07/08/2022, 2:14 AM
that's definitely helpful
w

witty-crayon-22786

07/08/2022, 2:33 AM
this ended up auto-merging a broken dependabot review… revert here: https://github.com/pantsbuild/pants/pull/16094
b

bitter-ability-32190

07/08/2022, 1:09 PM
FWIW I know this has caused a bit of heartburn but I think the idea is great! Once we work out the kinks I think this will be really good UX
h

happy-kitchen-89482

07/08/2022, 5:14 PM
Still struggling to get it to work 😞
b

bitter-ability-32190

07/08/2022, 6:09 PM
It seems like we're doing this for docs-only-CI-ness, right? Why not have "merge OK" need all the jobs, and those jobs conditionally choose if they get skipped or not?
h

happy-kitchen-89482

07/08/2022, 6:48 PM
That is already what happens
Merge OK needs all the jobs
b

bitter-ability-32190

07/08/2022, 6:48 PM
Then why do we need two merge ok jobs?
h

happy-kitchen-89482

07/08/2022, 6:48 PM
I guess you're saying since "skip" counts as "passed" we don't need two of the Merge OK jobs?
coke
We needed two under the incorrect assumption that "skip" counts as "failed"
But since that is wrong, you may be right
Gonna give it a try!
Can you give me a shipit on https://github.com/pantsbuild/pants/pull/16095 ? I won't actually merge, I want to see if GH would let me merge with a positive review
@bitter-ability-32190 you are a genius. This just works.
1
🙌 1
🧠 1
b

bitter-ability-32190

07/09/2022, 1:06 AM
Can we do something similar and eliminate
skip-rust
and
skip-build-wheels
"tags" 🙂
h

happy-kitchen-89482

07/09/2022, 1:06 AM
Oh, wait
I didn't reload the browser
DAMMIT
It doesn't work
b

bitter-ability-32190

07/09/2022, 1:06 AM
I'm still a genius tho, right?
h

happy-kitchen-89482

07/09/2022, 1:06 AM
E.g., https://github.com/pantsbuild/pants/pull/16095 can be merged even with failed jobs
because "Merge OK" is skipped due to its needs not being met, and apparently that counts as passing!
You are still a genius
GitHub Actions, less so
b

bitter-ability-32190

07/09/2022, 1:08 AM
That doesn't make sense why it would be skipped if it has no conditional
Do we have "run-the-action-from-the-branch" enabled? usually its disabled for security reasons
E.g. I make a PR which changes the action to mine some bitcoin and pass. Then I can merge and profit. Usually GitHub runs the action from
main
to avoid this.
h

happy-kitchen-89482

07/09/2022, 1:11 AM
I'm not sure
but is that relevant?
It makes sense that a job that needs other jobs to succeed before running would not run when they fail
b

bitter-ability-32190

07/09/2022, 1:12 AM
Yeah, becuase on your PR we'd still be running `main`'s workflow definitions
h

happy-kitchen-89482

07/09/2022, 1:12 AM
We're not
I know that my changes are running in the PR
b

bitter-ability-32190

07/09/2022, 1:13 AM
How so?
h

happy-kitchen-89482

07/09/2022, 1:13 AM
It makes no sense to me that if a job that needs other jobs to succeed doesn't run because they fail, then its own status is reported as success
WDYM? I can see them running
They print stuff out
I've added debug echo statements and all
b

bitter-ability-32190

07/09/2022, 1:14 AM
I only see one echo statement and its in the skipped job's workflow
its also malformed it seems 😅
Although that echo run looks weird
h

happy-kitchen-89482

07/09/2022, 1:56 AM
I assure you that my changes are the ones running
I have been iterating on them all day
What specifically is weird at that link?
I think I may have finally cracked it
OK, looking good so far
Yep, this finally works properly! https://github.com/pantsbuild/pants/pull/16095
b

bitter-ability-32190

07/09/2022, 9:53 AM
What was weird was the job was skipped, but had no conditional
h

happy-kitchen-89482

07/09/2022, 2:52 PM
It was skipped because it depended on other jobs that failed. Which as it turns out behaves like a conditional skip (so counts as passing).
b

bitter-ability-32190

07/09/2022, 2:53 PM
Woooow such bad UX
h

happy-kitchen-89482

07/09/2022, 2:58 PM
Pretty confusing
b

busy-vase-39202

07/11/2022, 4:46 PM
Seems worth opening an issue to GH. They do actually try with UX, so I think they'd care.
2 Views