-
Notifications
You must be signed in to change notification settings - Fork 238
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 CircleCI support #55
Conversation
Yay, thanks for the PR @jhersh! Regarding Coveralls + Circle CI, the folks at Coveralls are super helpful. I filed an issue here a few weeks ago and they helped me diagnose the issue really quickly, so it might be worth asking there too. If anyone else has suggestions, that’d be nice as well. Thanks again for the PR! |
Update! I believe the missing piece was the That's not ideal -- at least for Coveralls -- as it is a semi-secret key, so I've added that token as an environment variable in my CircleCI project and tweaked my pull request to look for that variable. My CircleCi project using my slather fork is now posting a build to Coveralls, but it shows up as 'no data' under each of my project files. Closer still... |
Woah! I just had a whole bunch of Coveralls builds show up, with per-file coverage and all. Perhaps their queues were backed up for a while? It looks like everything is showing up properly with the exception of the build branch, commit message, and author name. I think I'll need to add those to the Coveralls JSON payload and then we should be all set. |
We are close indeed! But I think this is not quite ready just yet - for one, the pull request should only be included in the JSON payload if there is actually a relevant pull request, as otherwise it shows up on Coveralls incorrectly as "pull request #0". I'll try to clean this up soon. |
Cool stuff @jhersh. I'll take a look at the code. Also, here's some relevant talk on the PS I'm using |
@@ -32,6 +53,19 @@ def coveralls_coverage_data | |||
else | |||
raise StandardError, "Environment variable `TRAVIS_JOB_ID` not set. Is this running on a travis build?" | |||
end | |||
elsif ci_service == :circleci | |||
if circleci_job_id |
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.
@ayanonagon we should probably look into moving these specific cases out into their own methods to cut down on clutter
looks good! just give a ping when it's ready |
@marklarr Awesome! I'd love to hear your thoughts sometime on SSDataSources :) I think we are in business! I removed my check for the |
@ayanonagon I'm going to turn off the coveralls PR commenter |
thanks @jhersh! Honestly, I don't have much to say about SSDataSources yet, and I'd say that's a good thing. I give it some objects and give it some blocks, and I get a table. Quick and easy |
Excellent! We would be keen on a new release of slather as soon as you are able :) Also I'm to blame for the coveralls spam; I squashed and force pushed a few times 😊 |
I've added support for CircleCI, a neat CI service we use at work and which I've increasingly been using for my own personal projects. Like Travis, they offer free build containers for FOSS projects.
Here is a sample CircleCI+slather build in one of my projects using my slather fork. The slathering, yea, it doth verily slather, but the build does not show on Coveralls. I am wondering if this is because:
I would appreciate any suggestions you may have and your help in getting this to the finish line. Thanks!