Closed
Bug 409394
Opened 17 years ago
Closed 16 years ago
support long version names for alphas/betas in Bootstrap
Categories
(Release Engineering :: General, defect, P2)
Release Engineering
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: bhearsum, Assigned: bhearsum)
References
Details
Attachments
(1 file, 1 obsolete file)
17.92 KB,
patch
|
nthomas
:
review+
|
Details | Diff | Splinter Review |
(I really wish I'd caught this in my original patch!) We've run into this problem in a couple of places now, Stage.pm, Updates.pm (specifically, bumping the config files properly). I'm thinking of abstracting this logic out into a couple of functions in Bootstrap::Config (one for current version, one for oldVersion). Both functions will take a binary argument of 'longName'. When false, they will return the version as it exists in bootstrap.cfg. When true, they will return, ex. "3.0 Beta 2". For non-alphas/betas the version will not change. What do you guys think?
Comment 1•17 years ago
|
||
I like putting this logic in one place; dont mind whether its Bootstrap::Config or Bootstrap::Util.
Assignee | ||
Updated•17 years ago
|
Priority: -- → P2
Assignee | ||
Updated•17 years ago
|
Priority: P2 → P3
Assignee | ||
Updated•17 years ago
|
Priority: P3 → P2
Assignee | ||
Comment 2•17 years ago
|
||
The patch adds long name support in places it was missed before and factors out the logic to two helper functions in Bootstrap::Config.
Attachment #294732 -
Flags: review?(nrthomas)
Comment 3•16 years ago
|
||
Comment on attachment 294732 [details] [diff] [review] support long names for alphas & betas Nice cleanup, and pretty close, but a few comments below. >Index: Bootstrap/Config.pm ... >+sub GetCurrentVersion { ... >+sub GetOldVersion { Could you modify these two functions to explicitly handle being called without the longName argument, either by specifying a default value or ASSERTing. Most functions in Config seem to do the latter. I'm also tempted to suggest changing GetCurrentVersion() to GetVersion(), to mimic what the vars are called. What do you think ? Do you remember finding the version/oldVersion distinction confusing when you first hit it ? >Index: Bootstrap/Step/Stage.pm ... >@@ -430,7 +430,7 @@ > my $product = $config->Get(var => 'product'); > my $appName = $config->Get(var => 'appName'); > my $logDir = $config->Get(sysvar => 'logDir'); >- my $version = $config->Get(var => 'version'); >+ my $version = $config->GetCurrentVersion(longName => 0); > my $rc = $config->Get(var => 'rc'); > my $stageHome = $config->Get(var => 'stageHome'); > my $productTag = $config->Get(var => 'productTag'); You can actually remove this one, it doesn't seem to be used in Verify().
Attachment #294732 -
Flags: review?(nrthomas) → review-
Assignee | ||
Comment 4•16 years ago
|
||
I have mixed feelings about the unspecific 'GetVersion', but in the interest of being consistent I've changed the function to GetVersion().
Attachment #294732 -
Attachment is obsolete: true
Attachment #295800 -
Flags: review?(nrthomas)
Comment 5•16 years ago
|
||
Comment on attachment 295800 [details] [diff] [review] rename GetCurrentVersion to GetVersion, add asserts r+, thanks Ben.
Attachment #295800 -
Flags: review?(nrthomas) → review+
Assignee | ||
Comment 6•16 years ago
|
||
Checking in Bootstrap/Config.pm; /cvsroot/mozilla/tools/release/Bootstrap/Config.pm,v <-- Config.pm new revision: 1.17; previous revision: 1.16 done Checking in Bootstrap/Util.pm; /cvsroot/mozilla/tools/release/Bootstrap/Util.pm,v <-- Util.pm new revision: 1.14; previous revision: 1.13 done Checking in Bootstrap/Step/Build.pm; /cvsroot/mozilla/tools/release/Bootstrap/Step/Build.pm,v <-- Build.pm new revision: 1.24; previous revision: 1.23 done Checking in Bootstrap/Step/PatcherConfig.pm; /cvsroot/mozilla/tools/release/Bootstrap/Step/PatcherConfig.pm,v <-- PatcherConfig.pm new revision: 1.12; previous revision: 1.11 done Checking in Bootstrap/Step/Repack.pm; /cvsroot/mozilla/tools/release/Bootstrap/Step/Repack.pm,v <-- Repack.pm new revision: 1.22; previous revision: 1.21 done Checking in Bootstrap/Step/Sign.pm; /cvsroot/mozilla/tools/release/Bootstrap/Step/Sign.pm,v <-- Sign.pm new revision: 1.10; previous revision: 1.9 done Checking in Bootstrap/Step/Source.pm; /cvsroot/mozilla/tools/release/Bootstrap/Step/Source.pm,v <-- Source.pm new revision: 1.13; previous revision: 1.12 done Checking in Bootstrap/Step/Stage.pm; /cvsroot/mozilla/tools/release/Bootstrap/Step/Stage.pm,v <-- Stage.pm new revision: 1.33; previous revision: 1.32 done Checking in Bootstrap/Step/TinderConfig.pm; /cvsroot/mozilla/tools/release/Bootstrap/Step/TinderConfig.pm,v <-- TinderConfig.pm new revision: 1.6; previous revision: 1.5 done Checking in Bootstrap/Step/Updates.pm; /cvsroot/mozilla/tools/release/Bootstrap/Step/Updates.pm,v <-- Updates.pm new revision: 1.32; previous revision: 1.31 done Checking in Bootstrap/Step/Tag/Bump.pm; /cvsroot/mozilla/tools/release/Bootstrap/Step/Tag/Bump.pm,v <-- Bump.pm new revision: 1.12; previous revision: 1.11 done
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
You need to log in
before you can comment on or make changes to this bug.
Description
•