Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Pull request to merge CodePorting service hook #289

Merged
merged 8 commits into from Apr 27, 2012

Conversation

Projects
None yet
2 participants
Contributor

CodePorting commented Apr 16, 2012

Please merge changes into guthub/github-services.

Member

technoweenie commented Apr 18, 2012

Needs tests. The unindented code isn't ideal, but I can fix that if you want to be a rebel :)

Contributor

CodePorting commented Apr 18, 2012

yeah, sure :)
To test we need the service to be active/live, don't we? Please suggest

Contributor

CodePorting commented Apr 19, 2012

Please have a look now, I have updated the service hook and uploaded test. Please merge changes or suggest what else is required.

Member

technoweenie commented Apr 20, 2012

Wow, this is intense. The services use Faraday, so you can probably delete most of that code. It should handle things like multipart for you.

Member

technoweenie commented Apr 20, 2012

This service needs to be dramatically simplified. GitHub Services typically don't do much work at all. At most, they parse the hook from GitHub.com and format a message to a 3rd party service. This is actually downloading the code locally, and then sending it to your service?

We do support other similar services, and they take the notification from GitHub Services and handle the code downloading/installing on their end.

Contributor

CodePorting commented Apr 23, 2012

Please check the service code, I have simplified the logic in service hook and placed rest of the functionality on our end. Please merge the changes if they are appropriate now, or suggest otherwise :)

Member

technoweenie commented Apr 23, 2012

I'd really appreciate it if you'd use Faraday. This keeps us from being tied to a single http adapter (net/http), and lets us run fast tests without hitting external services. Basically every server uses it.

CodeClimate is a great example. Most hooks just take the payload and send it on -- or mash it into a format for some existing API. Here's the documentation for sending a POST: #http_post.

Contributor

CodePorting commented Apr 23, 2012

Please have a look at code now, I have used the Faraday to make calls this time, Please let me know if anything is off the standards and I will update it according to your suggestions :)

@technoweenie technoweenie added a commit that referenced this pull request Apr 27, 2012

@technoweenie technoweenie Merge pull request #289 from CodePorting/master
Pull request to merge CodePorting service hook
c52e07f

@technoweenie technoweenie merged commit c52e07f into github:master Apr 27, 2012

Member

technoweenie commented Apr 27, 2012

Dang, accidentally merged this. This code was really bad and far from working. I got it passing tests though.

Contributor

CodePorting commented Apr 27, 2012

Please suggest so that I can make necessary changes to make the code perfectly acceptable for merging into github:master. I would really appreciate any help in this regard.

Member

technoweenie commented Apr 29, 2012

It's merged.

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