https://pantsbuild.org/ logo
Join Slack
Powered by
I'm going to restore the branch checks to just loo...
# development
h
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
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
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
can go look at the example i linked in the previous thread. it auto-merged after the first Merge OK
b
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
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
Yiiiiikes
b
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
that's definitely helpful
w
this ended up auto-merging a broken dependabot review… revert here: https://github.com/pantsbuild/pants/pull/16094
b
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
Still struggling to get it to work 😞
b
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
That is already what happens
Merge OK needs all the jobs
b
Then why do we need two merge ok jobs?
h
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
Can we do something similar and eliminate 
skip-rust
and 
skip-build-wheels
"tags" 🙂
h
Oh, wait
I didn't reload the browser
DAMMIT
It doesn't work
b
I'm still a genius tho, right?
h
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
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
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
Yeah, becuase on your PR we'd still be running `main`'s workflow definitions
h
We're not
I know that my changes are running in the PR
b
How so?
h
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
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
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
What was weird was the job was skipped, but had no conditional
h
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
Woooow such bad UX
h
Pretty confusing
b
Seems worth opening an issue to GH. They do actually try with UX, so I think they'd care.
2 Views
Open in Slack
PreviousNext
Powered by