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

[github_pr_destination] Don't require context reference if pr_branch is set #229

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

restingbull
Copy link

forked from #226 to address requested changes.

This will allow copybara to succeed for the following origin:

git.origin(
    url = "https://github.com/google/copybara.git",
    ref = "b0f6c6bbb5828c95b2b1409b4e491865d969f679",
)

Changes

Only fail on missing context reference when it is required.

  • This appears to be a reasonable compromise given that there are many ways to create a Revision without context reference.
  • Added appropriate tests.
  • Add null handling for Revision in pr branch name resolution (as it's marked with Nullable, however that works.)
  • Fail when labels other than CONTEXT_REFERENCE are used in the pr name. (bit of a bug -- previously, for any interpolated label would just reprint the label. Since no tests failed when fixed, it appears to be unexpected behaviour.)
  • Side note: these files seem to have missed the goggle-java-format boat. :[

cc: @Yannic, @petemounce

Yannic and others added 2 commits March 11, 2023 16:25
…` is set

This will allow copybara to succeed for the following origin:

```
git.origin(
    url = "https://github.com/google/copybara.git",
    ref = "b0f6c6bbb5828c95b2b1409b4e491865d969f679",
)
```
* This appears to be a reasonable compromise given that there are many ways to create a Revision without context reference.
* Add null handling for Revision in pr branch name resolution (as it's marked with Nullable, however *that* works.)
* Fail when labels other than CONTEXT_REFERENCE are used in the pr name. (bit of a bug -- previously, for any interpolated label would just reprint the label. Since no tests failed when fixed, it appears to be unexpected behaviour.)
* Side note: these files seem to have missed the goggle-java-format boat. :[
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