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

ci: ci improvements #2515

Merged
merged 2 commits into from
Nov 5, 2017
Merged

Conversation

tmc
Copy link
Contributor

@tmc tmc commented Nov 4, 2017

Status

Ready for review

Description of Changes

  • Make rebasing in ci be more specific about the upstream.
  • Skips ec2 tests in forks (ec2 tests will likely fail in forks anyways).
  • Perform rebasing in a separate job - this separation makes build failures more actionable.

Testing

circle job

Deployment

N/A

Checklist

N/A

@redshiftzero
Copy link
Contributor

Just talked with @tmc about this at the hackathon - while most of these changes are solid, we don't want to skip EC2 tests in forks and show green builds as it will lead maintainers to believe the tests ran successfully

@tmc tmc force-pushed the ci-improvements branch from 6a8be57 to 9a646ce Compare November 4, 2017 23:46
@redshiftzero
Copy link
Contributor

To get staging tests to pass here, @tmc disabled building of SecureDrop in his Circle CI account. Otherwise he was getting failures like this one, due to the use of his own account being used to run the Circle CI tests (and the expected EC2-related creds not being present).

Copy link
Contributor

@redshiftzero redshiftzero left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @tmc!

git fetch origin develop
git rebase origin/develop
git fetch upstream develop
git rebase upstream/develop
Copy link
Contributor

Choose a reason for hiding this comment

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

whoops, good call 👍

@@ -76,3 +120,6 @@ workflows:
- staging-test:
requires:
- docs-lint
- staging-test-with-rebase:
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that this is a good practice (running tests both on their own branch and rebasing on the target branch - this is what we do for Travis tests as well).

Copy link
Contributor

Choose a reason for hiding this comment

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

Though this increases costs so @msheiny/@conorsch happy to re-evaluate later this week

@redshiftzero redshiftzero merged commit b671c41 into freedomofpress:develop Nov 5, 2017
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