When HEAD is detached find GitHub PR by SHA

[drewish]

I've been trying to use the GithubPullRequestReviewFormatter with a Jenkins pipeline but ran into a problem. Jenkins doesn't expose the PR number and it checks out the commit from the webhook rather than the branch. The best fix I could come up with was modifying pronto to find the PR based off the SHA if the repo has a detached HEAD.

I wasn't sure if this would be an acceptable change so I didn't bother with any specs. If it's something you'd consider merging I'm happy to add tests.

[mmozuras]

@drewish seems good to me 👍

@mknapik @doomspork @aergonaut what do you think?

[aergonaut]

Seems alright to me. I don't think it's possible to have more than one PR with the same head SHA, so finding the PR based on that seems fine.

[drewish]

Cool I'll throw some specs together. Any suggestions on how to resolve that code climate issue?

[drewish]

Actually looking at the existing tests I'm not sure specs for this make a ton of sense. Pronto::Github#pull is private and Pronto::Git::Repository#head_detached? just delegates to the Rugged::Repository. If there's something that makes sense to the maintainers I'm happy to add it though.

[doomspork]

@mmozuras sounds like a good idea, I'd run into this issue myself w/ a previous client.

[aergonaut]

Codeclimate is complaining about the ABC size of that block. I personally think the block is fine as written so I would rather just increase the max value of that metric. @mmozuras what do you think about that?

[mknapik]

@drewish You might find it useful: https://github.com/u2i/jenkins-pipeline-utils/blob/master/src/me/knapik/GitHub.groovy
https://github.com/u2i/jenkins-pipeline-utils/blob/master/vars/foreachPullRequest.groovy

And an example usage:

        foreachPullRequest(GITHUB_ACCESS_TOKEN_CREDENTIALS_ID) { int pullRequestId, String targetBranchRef ->
            withEnv(["PULL_REQUEST_ID=${pullRequestId}"]) {
                rake "ci:pronto:pr[origin/${targetBranchRef}]"
            }
        }

Also Jenkins Pipeline has two types of jobs for GitHub org: branch job and PR job. I'm pretty sure PR job provide you with PR number in an environment variable.

If you'd like to use my jenkins pipeline shared library I can provide you with more info how to set it up.

Although, having it in Pronto would be so much better :-)

[drewish]

@mknapik oh neat yeah i've got some ruby code that does something similar. trying to do it in groovy with the CPS stuff was too much of a frustration for me. i found it easier to just call it via the shell. but when i was looking at the code in pronto i realized it just needed a couple tweaks to let me ditch my custom code.

Updated: If you know the name of an environment variable with the PR number in it please let me know. I'd spend up https://issues.jenkins-ci.org/browse/JENKINS-44948 against the GitHub Branch Source Plugin since it didn't seem like existing functionality.

[mknapik]

@drewish The name of the job for PR job type is PR-XX.

So it is possible to strip PR-. Not as straightforward though.

[drewish]

@mknapik ah thanks, I checked and we're doing the branch based PRs so that makes more sense to me now.

@mmozuras I can bump the ABC metric as part of this PR if you think that makes sense or I'm happy to do that in a separate PR if that needs to be a prerequisite for this.

[mmozuras]

@drewish we're in no hurry, so would prefer if you'd do it as part of this PR 👍

[drewish]

I bumped it up to 25 since that was showing ~21. It looks like we've resolved a few other places that were getting flagged for that complexity issue.

It's looking green!

[drewish]

@mmozuras let me know if there's anything else this PR needs.

[mmozuras]

@drewish nope, looks good! 👍

[mmozuras]

Released as part of v0.9.4.