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 Wercker support #290

Closed
wants to merge 2 commits into from
Closed

Add Wercker support #290

wants to merge 2 commits into from

Conversation

mrhyde
Copy link
Contributor

@mrhyde mrhyde commented Oct 26, 2014

Wercker do not provide public api yet so you can call it a "temporary workaround". Because of that build status is only available via Project ID (see example).

P.S. I wasn't able to test the code locally because of dependency hell (i really don't need all that stuff), so we need see what Travis have to say.

Resolves #265

@espadrine espadrine closed this in 66ad367 Oct 26, 2014
@espadrine
Copy link
Member

Thanks a lot!

I chose to use the format /wercker/ci/<id>.svg instead. Tell me if it is fine.

@mrhyde
Copy link
Contributor Author

mrhyde commented Oct 26, 2014

Personally i can't understand the use of /ci/ uri segment since none of other services have it (travis, codeship etc.)

@espadrine
Copy link
Member

I felt the same way at first. Then, my assumption that a particular brand wouldn't provide more than a single piece of information turned out wrong several times, and the corresponding code increase in complexity as a result.

I can't really change what I shipped, but I try very hard not to forget to add a vendor-specific switch in future URLs. You may notice that most of the services have one.

@mrhyde
Copy link
Contributor Author

mrhyde commented Oct 26, 2014

Well, i get your point.
I posted some issue regarding naming convention here
/issues/291

Thanks for accepting this pr so fast 👍

@mrhyde mrhyde deleted the wercker-integration branch October 26, 2014 22:29
@espadrine
Copy link
Member

I understand where you come from. It may have been a direction I'd have taken in the past.

The way I see it, Travis' URI design can easily go wrong, though. Were they to offer a side service, such as code coverage analysis, we would have a hard time offering it with a URI using the same REST paradigm. On the other hand, if we had started out with /travis/t/joyent/node.svg, we would only need to offer /travis/c/joyent/node.svg.

Besides, putting the resource at the end kind of assumes that every vendor uses the same username/project scheme, which is definitely not the case. Many of them have different requirements (Gratipay requires just the username, and code climate additionally requires some information about the git hosting service in use), and sometimes vary in the number of entries to provide (branches are an obvious example, but Travis also has private repositories with completely different requirements, for instance).

I don't have much regret from cases where I used a vendor switch. I do have regrets from those where I didn't use one, and ended up writing nightmarish regexps to have clean, but future-averse, URLs.

I suppose switching order would work, too:

/travis/joyent/node/t.svg
/travis/joyent/node/master/t.svg

Maybe that would have been better. Maybe I am missing something that would have made this harder to maintain. The system I rely on now seems to hold water, at any rate.

@mrhyde
Copy link
Contributor Author

mrhyde commented Oct 27, 2014

Can you please copy-paste this answer to the issue i mentioned before? This PR went far beyond original topic :)

By the way, i can't see wercker in production :( Are you using Continuous Delivery?

@mrhyde
Copy link
Contributor Author

mrhyde commented Oct 27, 2014

Just tested, its working but you forget to mention it on project page.

@espadrine
Copy link
Member

Yep. I usually leave a bit of time for discussion to unravel before making it public.

If all is good, I'll close #291 and make it public in 12h.

espadrine added a commit that referenced this pull request Oct 28, 2014
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.

Support for Wercker
2 participants