Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try to make mediasoup-worker-prebuild CI task only run in releases #1280

Closed

Conversation

ibc
Copy link
Member

@ibc ibc commented Dec 28, 2023

Fixes #1243

While it's true that this PR is not running mediasoup-worker-prebuild, I think it's not gonna work. I assume that given the documentation of softprops/action-gh-release.

Some info:

Bonus tracks:

@@ -44,29 +46,36 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v4
if: startsWith(github.ref, "refs/tags/")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not skipping the whole workflow instead? Skipping individual steps looks odd

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not skipping the whole workflow instead?

How to do that? 😀
That's exactly what I've been trying to do.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is if at job level as well: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idif

But if job is only triggered by tag, this check doesn't have any effect in the first place.

@@ -2,6 +2,8 @@ name: mediasoup-worker-prebuild

on:
push:
branches:
- v3
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you add v3 branch if you only want to run this workflow on releases? I believe these conditions are additive.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did assuming they are not additive...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

@nazar-pc nazar-pc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading #1243, I suspect you're pushing tag first and then you're creating release for that tag later. The error is confusing, but on GitHub you can only attach assets to releases and pre-releases, so if release it missing, upload will fail too. You need to change the workflow to be triggered by release, not tag. And probably exclude rust-* because it is not needed for those releases.

@@ -44,29 +46,36 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v4
if: startsWith(github.ref, "refs/tags/")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is if at job level as well: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idif

But if job is only triggered by tag, this check doesn't have any effect in the first place.

@@ -2,6 +2,8 @@ name: mediasoup-worker-prebuild

on:
push:
branches:
- v3
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ibc
Copy link
Member Author

ibc commented Dec 29, 2023

You need to change the workflow to be triggered by release, not tag. And probably exclude rust-*

So you mean that my first commit was way better?

61a6458

@nazar-pc
Copy link
Collaborator

nazar-pc commented Dec 29, 2023

So you mean that my first commit was way better?

I do think it should have been sufficient, also branches: [v3] was not necessary there.

BTW you can test how it performs by forking into personal repo and trying some tags/releases, from my experience that is sometimes the only way to find out how it works in practice.

@ibc
Copy link
Member Author

ibc commented Dec 29, 2023

@nazar-pc it looks that branches and tags are indeed acumulative, so I'll remove branches completely:

CleanShot-2023-12-29-at-10 22 43@2x

And yes, I¡ll do this in a fork.

@ibc ibc mentioned this pull request Dec 29, 2023
@ibc
Copy link
Member Author

ibc commented Dec 29, 2023

I'm closing this PR and trying things in a personal mediasoup fork. Cosmetic changes done here are now in a separate PR #1281.

@ibc ibc closed this Dec 29, 2023
@ibc
Copy link
Member Author

ibc commented Dec 29, 2023

@nazar-pc let's follow the discussion and experiments in the issue report: #1243 (comment)

@ibc ibc deleted the try-to-fix-mediasoup-worker-prebuild-ci-running-for-every-commit branch January 6, 2024 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Avoid that mediasoup-worker-prebuild CI task runs for every PR
2 participants