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

update github status to allow for drier object oriented usage #7

Merged

Conversation

jebentier
Copy link
Contributor

Doing this to help with dryness in pipeline files that are looking to set many statuses for the same sha

Copy link
Contributor

@jadametz jadametz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide a brief example of what calling this in a pipeline would look like?

@Invoca Invoca deleted a comment from jebentier Jul 10, 2018
@jebentier
Copy link
Contributor Author

Yes, example new usage is

githubStatusChecks = new GitHubStatus(this, '<repo>', env.GIT_SHA,  env.BUILD_URL, env.TOKEN)
githubStatusChecks.setStatus('jenkins-ci/production-assets-pushed', 'Assets pushed to S3', 'success')

or you can still use the old

gitHubStatus([
    repoSlug:    '<repo>',
    sha:         env.GIT_COMMIT,
    description: 'Assets pushed to S3',
    context:     'jenkins-ci/production-assets-pushed',
    targetURL:   env.BUILD_URL,
    token:       env.GITHUB_TOKEN,
    status:      'success'
])

@jebentier
Copy link
Contributor Author

is there a contrib anywhere on how to apply contributions and make them public?

}

void update() {
public void setStatus(String context, String status, String description) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHubStatus objects are meant to encapsulate an individual status. context is part of the composite key that identifies each status, so it should be included in the constructor.

Nit: public is the default in Groovy.

@jebentier jebentier force-pushed the 1807/eb/TECH-2610_add_secrets_check_to_jenkins_pipeline branch from 801ef20 to 2ba6f00 Compare July 10, 2018 20:06
@jebentier
Copy link
Contributor Author

@cmparkinson after an offline talk i went ahead and extracted the github api portion out into it's own object, which allows for things to be dried up. I also switched the instance method update to take the new status as apposed to having the status be a part of the GitHubStatus object. This feel like a better contract given our discussion about the idea of getting the current status from the API.

original execution of gitHubStatus variable remains working, and drier usage of GitHubStatus looks like:

github_status = new GitHubStatus([
    repoSlug:    '<repo>',
    sha:         env.GIT_COMMIT,
    description: 'Assets pushed to S3',
    context:     'jenkins-ci/production-assets-pushed',
    targetURL:   env.BUILD_URL,
    token:       env.GITHUB_TOKEN
])

github_status.update('success')
github_status.update('failure')

@jebentier jebentier force-pushed the 1807/eb/TECH-2610_add_secrets_check_to_jenkins_pipeline branch from 2ba6f00 to f40e35e Compare July 10, 2018 20:13
@jebentier jebentier force-pushed the 1807/eb/TECH-2610_add_secrets_check_to_jenkins_pipeline branch from a3c1399 to e4c74e0 Compare July 10, 2018 21:04
@jebentier jebentier force-pushed the 1807/eb/TECH-2610_add_secrets_check_to_jenkins_pipeline branch from c642318 to 51e2438 Compare July 10, 2018 21:37
@jebentier jebentier force-pushed the 1807/eb/TECH-2610_add_secrets_check_to_jenkins_pipeline branch from 3312751 to c61e796 Compare July 10, 2018 21:46
@jebentier jebentier force-pushed the 1807/eb/TECH-2610_add_secrets_check_to_jenkins_pipeline branch from c61e796 to b32d777 Compare July 10, 2018 21:47
@jebentier
Copy link
Contributor Author

@cmparkinson @jadametz am i good to merge this? and what's the release process?

@jadametz
Copy link
Contributor

Looks good to merge. No formally outlined contribution guidelines at the moment. To make it available it just needs to have a tag/release. Feel free to add release notes 😄

@jebentier jebentier merged commit 10110a3 into master Jul 20, 2018
@jebentier jebentier deleted the 1807/eb/TECH-2610_add_secrets_check_to_jenkins_pipeline branch July 20, 2018 20:17
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.

3 participants