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

Add ci checks for api [WIP] #183

Closed
wants to merge 4 commits into from
Closed

Conversation

scisco
Copy link
Collaborator

@scisco scisco commented Aug 16, 2018

No description provided.

@scisco scisco changed the title Add ci checks for api Add ci checks for api [WIP] Aug 17, 2018
@m-mohr
Copy link
Collaborator

m-mohr commented Aug 20, 2018

@scisco Is the CI broken? #186 and #185 for example don't get a result, which would block merging them.

@scisco
Copy link
Collaborator Author

scisco commented Aug 20, 2018

@m-mohr The CI in this PR is broken because my changes have an error that I have to fix.

In the other two PRs that you mentioned #186 and #185 the build has not started because the user who has committed does not have an account on CircleCI. You should login to circleci using your github account, then add a new commit to the PR (branch) which will start the build

@m-mohr
Copy link
Collaborator

m-mohr commented Aug 20, 2018

Thanks for the reply. That sounds overly complicated and was not mentioned as requirement on sprint 3 AFAIK. TravisCI doesn't require this. Can this be simplified or do we require all contributors to go through this steps? Also, I don't like to clutter the commit history...

@scisco
Copy link
Collaborator Author

scisco commented Aug 20, 2018

We could certainly switch to Travis. What I explained only applies to PRs from forked repos btw. I think Travis has the same issue though. The problem is that the CI environment needs to checkout the code, for which it needs the necessary permission. When you login, you give the CI access to the forked repo on your account.

The new commit is only needed for the first time, because it's not possible to trigger a build for commits in the past.

@matthewhanson
Copy link
Collaborator

Yeah I'm pretty sure the same problem exists in Travis because it needs to build an individuals repo when forked and isn't granted permission by default.

@scisco
Copy link
Collaborator Author

scisco commented Aug 27, 2018

This whole approach didn't work out the way I had imagined. Will close this and open a separate PR againt the dev branch.

@scisco scisco closed this Aug 27, 2018
@scisco scisco deleted the add_ci_checks_for_api branch August 27, 2018 15:24
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.

4 participants