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

Merged
merged 1 commit into from Mar 11, 2015

Conversation

Projects
None yet
4 participants
@jhersh
Contributor

jhersh commented Mar 9, 2015

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:

  • The latest CircleCI build for this project is 9, but the latest Travis build is 334, and perhaps Coveralls enforces increasing build number order?
  • A missing access token or some other authorization requirement?
  • Something else?

I would appreciate any suggestions you may have and your help in getting this to the finish line. Thanks!

@ayanonagon

This comment has been minimized.

Contributor

ayanonagon commented Mar 9, 2015

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! :octocat: 🎊

@jhersh

This comment has been minimized.

Contributor

jhersh commented Mar 10, 2015

Update! I believe the missing piece was the ci_access_token, which slather allows you to specify in .slather.yml (it's not clear to me from whence it comes on Travis builds).

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...

@jhersh jhersh referenced this pull request Mar 10, 2015

Merged

Switch to CircleCI #49

@coveralls

This comment has been minimized.

coveralls commented Mar 10, 2015

Coverage Status

Coverage increased (+0.05%) to 48.63% when pulling 8b33fb1 on jhersh:circleci into 0219ead on venmo:master.

@jhersh

This comment has been minimized.

Contributor

jhersh commented Mar 10, 2015

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.

@coveralls

This comment has been minimized.

coveralls commented Mar 11, 2015

Coverage Status

Coverage increased (+0.06%) to 48.64% when pulling 2e73f79 on jhersh:circleci into 0219ead on venmo:master.

@ayanonagon

This comment has been minimized.

Contributor

ayanonagon commented Mar 11, 2015

@jhersh That’s awesome!

@marklarr @kylef Wanna review the Ruby? Everything looks good through my Objective-C/Swift goggles. 😛

@jhersh

This comment has been minimized.

Contributor

jhersh commented Mar 11, 2015

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.

@marklarr

This comment has been minimized.

Contributor

marklarr commented Mar 11, 2015

Cool stuff @jhersh. I'll take a look at the code. Also, here's some relevant talk on the ci_access_token and why it's in the yml file #17 (comment)

PS I'm using SSDataSource in a personal project right now. It's so nice not having to deal with table logic anymore.

@@ -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

This comment has been minimized.

@marklarr

marklarr Mar 11, 2015

Contributor

@ayanonagon we should probably look into moving these specific cases out into their own methods to cut down on clutter

@marklarr

This comment has been minimized.

Contributor

marklarr commented Mar 11, 2015

looks good! just give a ping when it's ready

@coveralls

This comment has been minimized.

coveralls commented Mar 11, 2015

Coverage Status

Coverage increased (+0.06%) to 48.42% when pulling c7e7198 on jhersh:circleci into 8f3cd42 on venmo:master.

@jhersh

This comment has been minimized.

Contributor

jhersh commented Mar 11, 2015

@marklarr Awesome! I'd love to hear your thoughts sometime on SSDataSources :)

I think we are in business! I removed my check for the ci_access_token ENV variable, included the pull request parameter only if there is actually a pull request, and added the most recent commit message to the JSON payload. Things are looking pretty good in Coveralls.

/cc @ayanonagon @BillClinton @KimKardashian

@coveralls

This comment has been minimized.

coveralls commented Mar 11, 2015

Coverage Status

Coverage increased (+0.06%) to 48.42% when pulling 4fe8370 on jhersh:circleci into 8f3cd42 on venmo:master.

@marklarr

This comment has been minimized.

Contributor

marklarr commented Mar 11, 2015

@ayanonagon I'm going to turn off the coveralls PR commenter

marklarr added a commit that referenced this pull request Mar 11, 2015

Merge pull request #55 from jhersh/circleci
Add CircleCI support

@marklarr marklarr merged commit 1bbe1f9 into SlatherOrg:master Mar 11, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.06%) to 48.42%
Details
@marklarr

This comment has been minimized.

Contributor

marklarr commented Mar 11, 2015

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

@jhersh jhersh deleted the jhersh:circleci branch Mar 11, 2015

@jhersh

This comment has been minimized.

Contributor

jhersh commented Mar 11, 2015

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 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment