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

Support cross-repository branch reference #14

Merged
merged 1 commit into from
Sep 15, 2020
Merged

Support cross-repository branch reference #14

merged 1 commit into from
Sep 15, 2020

Conversation

RoKish
Copy link
Contributor

@RoKish RoKish commented Sep 14, 2020

Description

Currently, the action always namespace a branch name with a repository owner name (repo-owner:branch-name). Because of this, it's impossible to find a pull request created from a forked repository. This pull request changes this behaviour. The action will append the owner prefix only in the case branch name doesn't already have one (i.e. it doesn't contain :).

Copy link
Owner

@juliangruber juliangruber left a comment

Choose a reason for hiding this comment

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

What about instead of parsing the string, we add another input(s) for this?

@RoKish
Copy link
Contributor Author

RoKish commented Sep 15, 2020

Yeah, we can do that, but I don't really see any problems with using just one parameter. Branch names are not allowed to contain colons (:), so the current approach is safe and backward compatible. Moreover, I find this approach more user-friendly since the configuration is easier:

  1. The user doesn't need to look into the documentation to realize the difference between the parameters
  2. There's no need to define priority, i.e. what to do if two parameters are specified.

@juliangruber juliangruber merged commit 70d2945 into juliangruber:master Sep 15, 2020
@juliangruber
Copy link
Owner

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.

2 participants