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

Add optional "same branch only" input #7

Merged
merged 9 commits into from
Jul 11, 2020
Merged

Add optional "same branch only" input #7

merged 9 commits into from
Jul 11, 2020

Conversation

gavinsharp
Copy link
Contributor

@gavinsharp gavinsharp commented Jun 23, 2020

This adds a same-branch-only input which can be set to false if you want to wait on pending runs regardless of which branch they're from.

I was trying to use this action to prevent simultaneous deploys, but realized it would only work to prevent deploys from the same branch. In our setup all non-main branches deploy to the same environment, so I need to wait on any pending run, even if it is from a different branch.

(Even more ideal would be a way to say "wait for all main or non-main branches, depending on whether this is a run from main or non-main", but that seemed too complex and also maybe too unique to our particular deploy setup to be worth supporting.)

This parameter defaults to true to preserve existing behaviour.

@gavinsharp
Copy link
Contributor Author

gavinsharp commented Jun 23, 2020

It looks like passing undefined as the branch param to OctokitGitHub.runs is not behaving the way I would expect (finds no runs rather than finds all runs). Let me find a way to fix that.

@gavinsharp
Copy link
Contributor Author

It looks like passing undefined as the branch param to OctokitGitHub.runs is not behaving the way I would expect (finds no runs rather than finds all runs). Let me find a way to fix that.

Fixed in 53c876b.

src/input.ts Show resolved Hide resolved
@gavinsharp
Copy link
Contributor Author

I was trying to figure out a way to write a test to capture the actual functionality better, but since the underlying behaviour that matters (filtering by branch) lives in Octokit, testing a mocked Octokit didn't seem that useful.

@softprops
Copy link
Owner

I'm sorry. I've let this one go. If you'd still like this functionality can you rebase with latest version of the code? I can commit to taking a look this weekend.

@gavinsharp
Copy link
Contributor Author

I'm sorry. I've let this one go. If you'd still like this functionality can you rebase with latest version of the code? I can commit to taking a look this weekend.

No worries. I've merged in master (just required a rebuild for dist/index.js, merged cleanly otherwise)

@gavinsharp
Copy link
Contributor Author

I lied: trivial fix also needed for the new test (935b7e2)

@douglascamata
Copy link

I'm waiting for this, thanks! <3

@chenrui333
Copy link
Collaborator

This would be a great addon for terraform operations (which relies on shared resources)

@softprops softprops merged commit 26642b6 into softprops:master Jul 11, 2020
@softprops
Copy link
Owner

I'll update the tag for this today

@gavinsharp gavinsharp deleted the add_same_branch_only_input branch July 11, 2020 19:42
@softprops
Copy link
Owner

updated the latest tag with this change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants