-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Make fetchdep check out matching branch name #29601
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
Conversation
on a push to a branch.
elif [ -n "$BUILDKITE_PULL_REQUEST_BASE_BRANCH" ]; then | ||
clone $deforg $defrepo $BUILDKITE_PULL_REQUEST_BASE_BRANCH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're dropping buildkite support here can we also drop it elsewhere, e.g. L48-L50
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, grep says that's everything.
# (ie. the 'master' branch should use matching 'master' dependencies) | ||
base_or_branch=$GITHUB_BASE_REF | ||
if [[ "$GITHUB_EVENT_NAME" == "push" ]]; then | ||
base_or_branch=${GITHUB_REF} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't this what the block above is trying to do with TRY_BRANCH? This implies $head
isn't set as expected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like $head is only set by getPRInfo()
which is only run for PR builds, so I think that's all just for PRs.
on a push to a branch.
We previously checked out develop js-sdk even when building / testing mster. This would have worked usually, except when we have non-backwards compat changes on the js-sdk which we sometimes do with unstable interfaces (like sliding sync).
Alternative to #29596
Checklist
public
/exported
symbols have accurate TSDoc documentation.