Skip to content

Conversation

nickelsen
Copy link
Contributor

This will make the tests run on circleci.com, which has 4 free containers (essentially free parallel builds) for public projects. Circle will annotate each commit with the test-status, so either green or red.

We should not merge anything into the base branch that is red.

Currently, one test fails (something with apm) so I guess that test failure needs to be fixed before we can merge this. We might just update the expectation of the test to the actual value, since the software is already released. The alternative is to ignore the test for now, until there's time to fix it. I think fixing the underlying reason for the test failure is the best option, but also the most time-consuming so to me, it should not be in scope here.

@dsjoerg

@nickelsen
Copy link
Contributor Author

You can see the build history here for my fork - we would set up something similar for the ggtracker remote:
https://circleci.com/gh/nickelsen/sc2reader

@dsjoerg dsjoerg merged commit 32721d3 into ggtracker:upstream Jun 9, 2016
@dsjoerg
Copy link
Member

dsjoerg commented Jun 9, 2016

OK @nickelsen, just committed d5f5203 which marks that failing test as expected-to-fail.

@nickelsen
Copy link
Contributor Author

Great! @dsjoerg I think an admin needs to set up the build on circleci.com (it's really simple, just clicking add project, clicking sc2reader in ggtracker and clicking build project). I cannot do that for ggtracker.

PRs from forks will automatically be run, because this is a public project.

Should we make upstream a protected branch, so changes have to pass tests and go through PRs to be merged in? It really improves the information flow so everyone knows what is going in when.

@dsjoerg
Copy link
Member

dsjoerg commented Jun 9, 2016

I'm configuring it now

@nickelsen
Copy link
Contributor Author

Awesome! Thanks! 😃

@dsjoerg
Copy link
Member

dsjoerg commented Jun 9, 2016

Hey @nickelsen where's the documentation on how to make a branch protected? Can't find it

@dsjoerg
Copy link
Member

dsjoerg commented Jun 9, 2016

@nickelsen never mind, found it

@dsjoerg
Copy link
Member

dsjoerg commented Jun 9, 2016

@nickelsen I assumed you were still talking about CircleCI but I see now that it's a github thing

@dsjoerg
Copy link
Member

dsjoerg commented Jun 9, 2016

@nickelsen CircleCI integration is complete w00t thanks

@nickelsen
Copy link
Contributor Author

Awesome - thank you! 👍 We should do the same for the other projects. I tried setting up circleci for ggpyjobs, but got blocked by S3 credentials - do you know if there's a way to test without that being set up? We could set it up in environment variables, but they will not be loaded for PRs from forks, because of security reasons.

@dsjoerg
Copy link
Member

dsjoerg commented Jun 9, 2016

@nickelsen with some minor changes, we can set the tests to get AWS/S3 credentials from environment variables, and there's a way to config CircleCI to have private environment variables. Then tests can use an AWS/S3 account that I provide for that purpose.

However given our tight-knit small group of contributors I'm inclined to save this for later.

@nickelsen
Copy link
Contributor Author

Ok, I understand - let's prioritise fixing the app for now. 😁

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