#284 emailing commiter on PRs from jenkins
Merged by lholecek. Opened by jmolet.
jmolet/greenwave improved-email  into  master

Download 284.patch

Is this roughly what y'all were thinking?

Interesting side effect I've noticed since we've added the builds against the PRs:

with this logic:

    if (previousResult == 'FAILURE' && currentResult == 'SUCCESS') {
        SUBJECT = "Jenkins job ${env.JOB_NAME} #${env.BUILD_NUMBER} fixed."
    }
    else if (previousResult == 'SUCCESS' && currentResult == 'FAILURE' ) {
        SUBJECT = "Jenkins job ${env.JOB_NAME} #${env.BUILD_NUMBER} failed."
    }

This logic worked because we built against the same branch over and over. Now that we have PRs and master builds all intermingled, there isn't much useful in comparing previousResult vs currentResult. As it is now, for example, if build N against PR 123 fails, and build N+1 against PR 456 passes, the author of 456 gets a "build fixed" email.

Ideally to counter this we would need to be able to get the previousResult for a given branch, but I'm short of ideas of a good way do that in a low failure prone way.

Other option would be to rely on the new recipient logic:

    def RECIEPENT = scmVars.GIT_AUTHOR_EMAIL
    if (ownership.job.ownershipEnabled && branch == 'master') {
        RECIEPENT = ownership.job.primaryOwnerEmail
    }

and planning on most dev work happening outside of master... but I don't have a good feel for this since I'm not a main dev for the project. Open to suggestions of what to implement :)

Would it be possibly to set name for the build and retrieve it later? (The name could be just the remote branch name.)

Looks like it could be possible with code from here.

Wouldn't it be more correct to check who did submit the PR?

I would expect pagure to have an endpoint for telling that, although didn't check.

@lholecek has a good point here. This could be similar to what Travis CI does.

@lholecek I will try this...

@csomh Messing around with this: https://pagure.io/api/0/ I can obtain the username of the person who submitted the PR, but the API never returns the user's email address ( neither at GET /api/0/user/ or GET /api/0//pull-request/ ). It doesn't seem to even return my own email address using my personal token.

@jmolet the API not exposing email address might actually make sense...

1 new commit added

  • Setting display name

1 new commit added

  • now with even fancier build descriptions

@lholecek build name and descriptions are now set with this.

The question about the email notifications remains. My recommendation:
- Team list gets all failure notifications against the master branch
- Commit author gets all failure and success notifications against PR branches

Thoughts?

+1 from me on that, @jmolet.

rebased onto 01d8d76fe69f65fae7fd80ab75e183eb67f6b996

Thanks @ralph , implemented that. Removing the [WIP] tag. Rebased onto master. If the tests pass this is good to merge from my perspective.

rebased onto d3f0626ff7db004aadac9f3307f102ec2cddb345

+1

Having the link to PR is really nice bonus.

Though, the build for this PR failed for some reason (looks like it sends e-mail correctly to just the author now).

Started a rebuild to see if it was just a blip or not.

Rebuilding picked #285 to build, and of course it did... @jmolet please force push this PR. Thanks!

@csomh I don't have commit access to this repo :)

@jmolet I think @csomh meant git commit --amend --no-edit && git push --force so that Jenkins job runs again.

Anyway, I'm merging this since it shouldn't fail.

Pull-Request has been merged by lholecek

Metadata