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

Context Awareness: Implement commit status contexts #161

Merged
merged 1 commit into from
Jan 9, 2015

Conversation

farmdawgnation
Copy link
Contributor

Back in March, Github released the Combined status API. As it nears the end of its developer preview period, the organization I work for has found need to start leveraging them. From what I can tell, this is working as intended. But GitHub doesn't seem to be reflecting these in their UI yet. 😢

That said, I think this could still be useful for teams that want it so I decided to go ahead and open the pull request. This PR can't be merged until release 1.55 of the github-api jenkins plugin is out the door. The java dependency itself has been released, but it looks like when they tried to release the plugin things didn't go as planned? I've emailed the author about it, and am still waiting on a response.

So, things we need:

  • github-api-plugin 1.55 being released.
  • Another pass through the tests, because I'm about 99% sure my brain isn't functioning correctly today. heh.

closes #155

@valdisrigdon
Copy link
Collaborator

@farmdawgnation This looks great! Once the dependent plugin is updated, I'll try to get this merged.

@farmdawgnation
Copy link
Contributor Author

Ok, the plugin is bumped but tests didn't pass for me locally when I ran them this morning. Will hopefully take a look at that soon-like.

@vorburger
Copy link
Contributor

Would integrating this allow one to run two different Jenkins jobs for each PR, and see the status of both of them? When I try that with the current version, the last one that ran just overwrites the first one (which makes it infeasible / near useless to set-up two jobs).

If so, then this would be great to have - the use case is e.g. running a Maven build for a corp. OSS project twice from different jobs - against both a public Maven central as well as against an internal mirror (to double check that's set-up correctly).

Anything holding back merging this? I see github-api v1.58 (>1.55 quoted as required above) is available now... and https://developer.github.com/changes/2014-03-27-combined-status-api/ from March mentions "preview period to last 30-60 days", and reading https://developer.github.com/v3/repos/statuses/ makes me thinkg that meanwhile this is part of the GitHub API. So... could this be merged today?

@farmdawgnation
Copy link
Contributor Author

Would integrating this allow one to run two different Jenkins jobs for each PR, and see the status of both of them?

When @github updates their UI... currently the UI doesn't listen to this. :/ But yes, that's the idea.

@bergman
Copy link

bergman commented Oct 22, 2014

Any new on this? Has @github updated their UI to include context yet?

@farmdawgnation
Copy link
Contributor Author

Disappointingly, no. :/

@ypresto
Copy link

ypresto commented Oct 28, 2014

I also want to just disable updating status, to keep from overwriting another status.

@fbernier
Copy link

fbernier commented Dec 9, 2014

@farmdawgnation
Copy link
Contributor Author

!!!!!!

Yes, yes it is!

I'll get this tidied up and ready to ship today.

@farmdawgnation
Copy link
Contributor Author

Ok, I think I picked the correct merge conflict resolution but I've got tests exploding in fiery sparks of doom. A bunch of invocation target exceptions. It looks like the pull request merge test may be the root of them, but I can't find any line numbers from that spec in the stack traces. Any chance someone has an idea of what's up?

@tucksaun
Copy link
Contributor

Hi,

For the tests, I only got failures due to encoding and Github rate limit on the API.

And code looked good so I gave it a try, it looks like everything works well here so I just deployed it on our CI server (we really needed this feature !!!)

So thanks for the work!

@farmdawgnation
Copy link
Contributor Author

Awesome! I'm going to see if I can get all greens on my end and deploy it to mine as well. Will post back here after I do!

@tucksaun
Copy link
Contributor

Actually @farmdawgnation there was others errors but I have found where tests were breaking for those ones, now the build is marked as success, I send you a PR to your fork ;)

@farmdawgnation
Copy link
Contributor Author

!!

Fixed configuration fixtures for commit status context
@farmdawgnation farmdawgnation changed the title [WIP] Context Awareness: Implement commit status contexts Context Awareness: Implement commit status contexts Dec 12, 2014
@farmdawgnation
Copy link
Contributor Author

I'm still seeing a bunch of NPE's when I try to run tests, but if that's only me then so be it. I think this is ready to go.

@bugraokcu
Copy link

Any news on this? Would be great to have this!

@farmdawgnation
Copy link
Contributor Author

We're using this at my office and it's been working great! Ready to see it in the main plugin! :)

@felixmohnert
Copy link

It would be really great to have this soon. Thanks for the implementation @farmdawgnation !

@farmdawgnation
Copy link
Contributor Author

@valdisrigdon Can we get some attn here? :)

@akoeplinger
Copy link
Contributor

Isn't the primary repo on https://github.com/jenkinsci/ghprb-plugin now? I see @DavidTanner made a commit there one day ago.

@farmdawgnation
Copy link
Contributor Author

Well it would have been nice for someone to tell us. heh

@DavidTanner
Copy link
Collaborator

I will check in on this tonight/tomorrow. Sorry, I changed roles so I am not as active with jenkins.

@DavidTanner DavidTanner merged commit 81132cb into janinko:master Jan 9, 2015
DavidTanner added a commit that referenced this pull request Jan 9, 2015
This closes #161 All tests are passing again, and the latest from the other repo are merged in.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for commit status contexts