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

(FM-6170) Addition of branch check for build number creation #190

Merged
merged 1 commit into from
May 9, 2017

Conversation

HelenCampbell
Copy link
Contributor

The build ID on the release pipelines were occasionally causing the builds to break due to having the same or an already existing build number on master job. This PR provides the addition of a simple branch check followed by adding an 'r' to the release branch jobs - ensuring the conflict doesn't happen again.

@HelenCampbell HelenCampbell force-pushed the cibranchupdate branch 2 times, most recently from c151dd2 to 5f8e4c4 Compare May 8, 2017 15:39
@DavidS
Copy link
Contributor

DavidS commented May 8, 2017

Please use git rev-parse --symbolic-full-name HEAD instead of git branch to avoid depending on git human readable "porcelain" output.

@@ -500,10 +500,15 @@ def max_thread_limit
end

sha = `git rev-parse HEAD`[0..7]
branch = `git branch`
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be better to use git rev-parse --abbrev-ref HEAD

Doesn't require any magic text parsing.


# If we're in a CI environment include our build number
if build = ENV['BUILD_NUMBER'] || ENV['TRAVIS_BUILD_NUMBER']
new_version = sprintf('%s-%04d-%s', version, build, sha)
if branch.include? "* release"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we really be special casing release branch? Can we make this generic?

e.g. for master, append nothing. for any other branch append, say the first 7 letters or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think there's any reason to make this generic? We only ever juggle the two branches on CI. If someone else adds another job they'll need to decide for themselves if it requires the amendment. Though I'll maybe add a comment saying why it's needed so others know what's going on here :)

@puppetcla
Copy link

Waiting for CLA signature by @HelenCampbell

@HelenCampbell - We require a Contributor License Agreement (CLA) for people who contribute to Puppet, but we have an easy click-through license with instructions, which is available at https://cla.puppet.com/

Note: if your contribution is trivial and you think it may be exempt from the CLA, please post a short reply to this comment with details. http://docs.puppet.com/community/trivial_patch_exemption.html

@HelenCampbell
Copy link
Contributor Author

@glennsarti @DavidS

@glennsarti glennsarti merged commit 8443c1e into puppetlabs:master May 9, 2017
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.

5 participants