Closed
Bug 1365734
Opened 7 years ago
Closed 7 years ago
Handle BMO version number in Makefile.PL / MYMETA.json
Categories
(bugzilla.mozilla.org :: General, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: dylan, Assigned: dylan)
References
Details
Attachments
(1 file)
4.94 KB,
patch
|
glob
:
review+
|
Details | Diff | Splinter Review |
Neither the upstream method (BUGZILLA_VERSION in Bugzilla::Constants) nor the BMO customization (setting a parameter for the version number) are acceptable for BMO in the near future. 1. The version will be defined in Bugzilla.pm, as our $VERSION = 'YYYYMMDD.xx'. 2) This will be extracted by Makefile.PL and written into MYMETA.json 3) version.json will pull this from that file. 4) templates will also pull it from this file. 5) the version will be updated with a commit and that commit tagged for all future deployments.
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8870406 -
Flags: review?(glob)
(In reply to Dylan Hardison [:dylan] (he/him) from comment #0) > Neither the upstream method (BUGZILLA_VERSION in Bugzilla::Constants) > nor the BMO customization (setting a parameter for the version number) are > acceptable for BMO in the near future. it seems to me that if you're reworking this code, it would be better to set it automatically when checksetup is run (or similar), so the version is always bound with the deployment date without needing to remember to update Bugzilla.pm.
Assignee | ||
Comment 3•7 years ago
|
||
I'd rather it work like any other perl package. As we move to a more cloudy deployment, I will start tagging releases as well.
Comment on attachment 8870406 [details] [diff] [review] 1365734_1.patch Review of attachment 8870406 [details] [diff] [review]: ----------------------------------------------------------------- r=glob these changes look good as is, however i'm concerned that it would be too easy to forgot to update the version number before deployment, requiring a second push to address it -- the version being part of index's etag makes this more impactful than just a visible string. would it be possible/valuable to add a test that warns when the version hasn't been updated?
Attachment #8870406 -
Flags: review?(glob) → review+
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to Byron Jones ‹:glob› from comment #4) > would it be possible/valuable to add a test that warns when the version > hasn't been updated? Possible, yes, but with trade-offs: 1) I could base it on the date (if the version is not a future date, disallow) but that seems fragile 2) I could try connecting to github and seeing if the current branch's version is the same as production, and fail then. This requires network access and seems fragile for other reasons 3) I could locally run git to determine this -- but I'd need to make sure the production branch was pulled down. The problem all boils down to state. I don't mean to hand-wave away, but I think it is better to update the process documentation and live with the risk for now. Some of the constraints we're under now won't exist when deployment happens in a more containerized fashion.
Assignee | ||
Comment 6•7 years ago
|
||
... we're under now won't exist when deployment happens in a more containerized fashion, and we can revisit how the version is updated. Perhaps using git metadata, like tags. Or have CI ensure that the tag/version must match, etc.
Assignee | ||
Comment 7•7 years ago
|
||
To git@github.com:mozilla-bteam/bmo.git 5d6b042..9701901 master -> master
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 8•7 years ago
|
||
Documentation updated: https://mana.mozilla.org/wiki/pages/diffpagesbyversion.action?pageId=52266989&selectedPageVersions=12&selectedPageVersions=11
(In reply to Dylan Hardison [:dylan] (he/him) from comment #5) > I don't mean to hand-wave away, but I think it is better to update the > process documentation and live with the risk for now. Some of the > constraints we're under now won't exist when deployment happens in a > more containerized fashion. makes sense; thanks for the response.
You need to log in
before you can comment on or make changes to this bug.
Description
•