Skip to content
This repository was archived by the owner on Aug 29, 2023. It is now read-only.

fix: fix js test and release workflow#495

Merged
achingbrain merged 1 commit into
masterfrom
fix-js-build
Mar 15, 2023
Merged

fix: fix js test and release workflow#495
achingbrain merged 1 commit into
masterfrom
fix-js-build

Conversation

@achingbrain

@achingbrain achingbrain commented Mar 15, 2023

Copy link
Copy Markdown
Contributor

Fixes yaml syntax errors introduced in #487

The automerge jobs for this failed yet the PRs were still merged.

Fixes yaml syntax errors.

The automerge jobs for this [failed](https://github.com/ipfs/helia-ipns/actions/runs/4424888455)
yet the [PRs were still merged](ipfs/helia-ipns#9).
@achingbrain achingbrain requested a review from a team as a code owner March 15, 2023 11:10
@achingbrain achingbrain requested a review from galargh March 15, 2023 11:10
@achingbrain

Copy link
Copy Markdown
Contributor Author

I'm going to merge this because everything's on fire. @galargh the auto-merge job should probably not succeed if the actions it starts fail.

@achingbrain achingbrain merged commit 88629c9 into master Mar 15, 2023
@achingbrain achingbrain deleted the fix-js-build branch March 15, 2023 11:16
@galargh

galargh commented Mar 15, 2023

Copy link
Copy Markdown
Contributor

I'm going to merge this because everything's on fire. @galargh the auto-merge job should probably not succeed if the actions it starts fail.

I want to get rid of per-repo automerge workflows altogether - #477

@achingbrain

Copy link
Copy Markdown
Contributor Author

Sounds good, though I'm still confused as to why the change was merged when the sync builds failed.

@galargh

galargh commented Mar 15, 2023

Copy link
Copy Markdown
Contributor

It's because it didn't really fail. Automerge waits for all the checks running on a PR to finish and then merges the PR if all of them passed or were skipped. In this case, there were no checks on the PR because we broke the underlying workflow YML. If the YML is broken, GitHub has no idea what the workflow is supposed to do. In particular, it doesn't know that the YML was supposed to define checks for a PR.

@galargh

galargh commented Mar 15, 2023

Copy link
Copy Markdown
Contributor

Sorry I missed the issue in the review. I read through the comments too quickly and thought - #487 (comment) - was mentioning that we did try out this workflow in some repo already.

@achingbrain

Copy link
Copy Markdown
Contributor Author

It's because it didn't really fail.

That's interesting, because the build status is "Failure", maybe this is another status it should check for?

@galargh

galargh commented Mar 15, 2023

Copy link
Copy Markdown
Contributor

Yes, but sadly it's not happening in the context of the PR. However, now that I think of it, it is associated with a specific commit and GitHub is perfectly happy attaching checks from worklow runs triggered on commit pushes to PRs. Following that logic, it should also associate a faulty workflow definition from a commit with a PR that the commit is a part of. I'm going to raise this with Github support.

Screenshot 2023-03-15 at 13 58 37

@galargh

galargh commented Mar 15, 2023

Copy link
Copy Markdown
Contributor

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants