Skip to content

allow circle to build for remote branches - #492

Merged
bulldozer-bot[bot] merged 6 commits into
palantir:masterfrom
yifeih:test-ci-fork
Feb 20, 2019
Merged

bulldozer-bot[bot] merged 6 commits into
palantir:masterfrom
yifeih:test-ci-fork

Conversation

@yifeih

@yifeih yifeih commented Feb 20, 2019

Copy link
Copy Markdown

Testing whether circle CI starts...

@yifeih yifeih changed the title test circle fork allow circle to build for remote branches Feb 20, 2019
@yifeih

yifeih commented Feb 20, 2019

Copy link
Copy Markdown
Author

@mccheah @dansanduleac
Not sure why this one failed https://circleci.com/gh/palantir/spark/11108 (it failed to check out the commit hash) but it's been working ever since...? I also couldn't reproduce that error because a rerun of that build worked just fine.

I could keep pushing changes to trigger more builds if that helps?

@robert3005

robert3005 commented Feb 20, 2019 via email

Copy link
Copy Markdown

@yifeih

yifeih commented Feb 20, 2019

Copy link
Copy Markdown
Author

Do you mean "Build forked pull requests" on this page? https://circleci.com/gh/palantir/spark/edit#advanced-settings. That was already on before I started this PR

@robert3005

robert3005 commented Feb 20, 2019 via email

Copy link
Copy Markdown

@yifeih

yifeih commented Feb 20, 2019

Copy link
Copy Markdown
Author

Yea but it was consistently failing to fetch remote branches... i think it's because we overrode the circle checkout code. the regular code looks different. it doesn't have the branch environment variable (see "checkout" step here: https://circleci.com/gh/palantir/conjure-java-runtime/3543?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link)

@yifeih

yifeih commented Feb 20, 2019

Copy link
Copy Markdown
Author

But this seems to work now. Let me push a few more changes or start a new PR just to confirm

@yifeih

yifeih commented Feb 20, 2019

Copy link
Copy Markdown
Author

checkout seems to be working on new PRs, not sure what caused the previous flake: #494

@mccheah mccheah left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Think this is fine - @robert3005 would there be any reason not to introduce this anyway? Seems like it fixes some fork-based PRs.

@bulldozer-bot
bulldozer-bot Bot merged commit e0c176c into palantir:master Feb 20, 2019
yifeih added a commit that referenced this pull request Feb 20, 2019
@mccheah

mccheah commented Feb 20, 2019

Copy link
Copy Markdown

Think this is fine - @robert3005 would there be any reason not to introduce this anyway? Seems like it fixes some fork-based PRs.

Whoops, this broke the build so...

bulldozer-bot Bot pushed a commit that referenced this pull request Feb 21, 2019
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.

3 participants