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

Stop updating package.json version field during deployments from Jenkins #922

Merged
merged 6 commits into from
Feb 5, 2019

Conversation

robertknight
Copy link
Member

This PR removes updating of the the version field in package.json during a Jenkins deployment. Instead the version field is set based on the highest-tagged previous version when building the client and publishing it to npm. This change is not actually committed however.

The upshot is that, since client deployments no longer try to push additional commits to the master branch, it is possible to deploy client releases which are no longer the latest. This means that if there are a queue of recently being merged PRs waiting to be deployed, it is possible to deploy them incrementally rather than all at once.

Fixes #810

@codecov
Copy link

codecov bot commented Feb 4, 2019

Codecov Report

Merging #922 into master will decrease coverage by 0.2%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #922      +/-   ##
==========================================
- Coverage    92.1%   91.89%   -0.21%     
==========================================
  Files         142      142              
  Lines        5715     5715              
  Branches      900      900              
==========================================
- Hits         5264     5252      -12     
- Misses        451      463      +12
Impacted Files Coverage Δ
src/sidebar/components/thread-list.js 59.61% <0%> (-23.08%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5046b2a...3011665. Read the comment docs.

@codecov
Copy link

codecov bot commented Feb 4, 2019

Codecov Report

Merging #922 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #922      +/-   ##
==========================================
+ Coverage   91.88%   91.89%   +0.01%     
==========================================
  Files         142      142              
  Lines        5715     5715              
  Branches      900      900              
==========================================
+ Hits         5251     5252       +1     
+ Misses        464      463       -1
Impacted Files Coverage Δ
src/sidebar/components/markdown.js 93.15% <0%> (+1.36%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e971226...fc2cc58. Read the comment docs.

Creating a commit which updates the version number as part of
the release process prevents deployments of the master branch which are
not the latest commit, which is a surprising limitation compared to our
release process for h etc.

This commit resolves the issue by removing the version-bumping commit.
Instead package.json in the source tree contains a fixed, unchanging
version number which is set to the real version just before the package
is published to npm. The release version is obtained by fetching the tag
with the highest version number and incrementing the major part.
The call to `new IllegalArgumentException` fails inside the sandbox that
the Jenkinsfile script runs in. Use `error` instead per
https://stackoverflow.com/questions/42718785/
The package version is now determined based on the highest
previously-tagged version. This means we need to fetch existing tags
before we can determine the new version number.
The npm tag we wait on needs to match the one which was used when the
release was published.
`yarn version` does not run the "postversion" script when called with
`--no-git-tag-version`.
The "Build" step stopped working after b778c9a because `make` with no
arguments just prints help output.

Since the client is rebuilt with a different version number as part of the
QA and prod releases, a separate "Build" step serves no purpose.
@robertknight robertknight force-pushed the remove-package-version-updates branch from 5ed4747 to fc2cc58 Compare February 4, 2019 14:49
@@ -108,21 +103,13 @@ node {
"""
}
}

// Revert back to the pre-QA commit.
sh "git checkout ${lastCommitHash}"
Copy link
Member Author

Choose a reason for hiding this comment

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

No git commit is created during the QA release process, so this is unnecessary

@@ -178,7 +162,7 @@ node {
String bumpMinorVersion(String version) {
def parts = version.tokenize('.')
if (parts.size() != 3) {
throw new IllegalArgumentException("${version} is not a valid MAJOR.MINOR.PATCH version")
error "${version} is not a valid MAJOR.MINOR.PATCH version"
Copy link
Member Author

Choose a reason for hiding this comment

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

The previous code never worked in Jenkins because you can't new things in this script due to security restrictions.

@@ -148,7 +148,6 @@
"test": "gulp test",
"report-coverage": "codecov -f coverage/coverage-final.json",
"version": "make clean build/manifest.json",
"postversion": "./scripts/postversion.sh",
Copy link
Member Author

Choose a reason for hiding this comment

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

The logic that used to be here has been moved to the Jenkinsfile.

returnStdout: true
).trim()

stage('Build') {
Copy link
Member Author

Choose a reason for hiding this comment

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

This stage doesn't actually do anything useful. make on its own just prints help output. The client actually gets built during the "QA" and "prod" steps with the appropriate version number. If there is eg. a syntax error, it'll get flagged either during the Test or QA steps.

@robertknight robertknight removed the WIP label Feb 4, 2019
Copy link

@dmfine dmfine left a comment

Choose a reason for hiding this comment

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

Looks generally sensible, ready to test on a real deployment

@robertknight robertknight merged commit 475e7da into master Feb 5, 2019
@robertknight robertknight deleted the remove-package-version-updates branch February 5, 2019 06:51
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.

2 participants