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 source_pr_number input #434

Merged
merged 8 commits into from
Aug 21, 2024

Conversation

chancez
Copy link
Contributor

@chancez chancez commented Jul 25, 2024

Fixes #433

I've tested this in the repo I'm using this with and its been working well for manually triggering on some PRs that we merged prior to introducing the GHA.

chancez and others added 3 commits July 25, 2024 10:47
Fixes korthout#433

Signed-off-by: Chance Zibolski <chance.zibolski@gmail.com>
Currently the inputs are ordered alphabetically (I know underscores
aren't actually part of the alphabet). So, let's maintain this ordering.
Note that this is not specific to the workflow_dispatch event type. It
should also work for other event types.
Copy link
Owner

@korthout korthout left a comment

Choose a reason for hiding this comment

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

Thanks @chancez for this great contribution!

I finally had some time to look at open pull requests here. Sorry for the long wait 🙇

I really like the feature and will gladly support it. There's just one question remaining for me: why should this only be supported for workflow_dispatch?

EDIT: I've taken the liberty to remove this restriction already, but if you see a reason for this we can take away these commits again. Let me know what you think.

src/backport.ts Outdated
Comment on lines 108 to 113
if (
this.config.source_pr_number !== undefined &&
this.github.getEventName() !== "workflow_dispatch"
) {
throw new Error(
"source_pr_number can only be specified for workflow_dispatch events!",
Copy link
Owner

Choose a reason for hiding this comment

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

❓ Why only for this event type? Do you see a specific reason to not allow this for other event types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is fine. I believe I was just being cautious.

As far as I can see, there is no reason to limit the source_pr_number
input only to usage with workflow_dispatch events. Other workflows may
also benefit from this input if they want to specify the pull request.
@chancez
Copy link
Contributor Author

chancez commented Aug 21, 2024

Thanks @chancez for this great contribution!

I finally had some time to look at open pull requests here. Sorry for the long wait 🙇

I really like the feature and will gladly support it. There's just one question remaining for me: why should this only be supported for workflow_dispatch?

EDIT: I've taken the liberty to remove this restriction already, but if you see a reason for this we can take away these commits again. Let me know what you think.

I don't see any big reason, I think I just was being cautious, because I think I remember seeing the GHA already checking the event type in another case, so I think I just copied it. I can't remember exactly though.

Copy link
Owner

@korthout korthout left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@korthout korthout merged commit 6610ffe into korthout:main Aug 21, 2024
1 check passed
@korthout korthout changed the title Add source_pr_number input for use with workflow_dispatch events. Add source_pr_number input Aug 21, 2024
@korthout
Copy link
Owner

Released in v3.1.0

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.

Workflow dispatch support
2 participants