support long version names for alphas/betas in Bootstrap

RESOLVED FIXED

Status

defect
P2
normal
RESOLVED FIXED
12 years ago
6 years ago

People

(Reporter: bhearsum, Assigned: bhearsum)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(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?
I like putting this logic in one place; dont mind whether its Bootstrap::Config or Bootstrap::Util. 
Assignee

Updated

12 years ago
Priority: -- → P2
Assignee

Updated

12 years ago
Priority: P2 → P3
Assignee

Updated

12 years ago
Priority: P3 → P2
Assignee

Comment 2

12 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 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

12 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 on attachment 295800 [details] [diff] [review]
rename GetCurrentVersion to GetVersion, add asserts

r+, thanks Ben.
Attachment #295800 - Flags: review?(nrthomas) → review+
Assignee

Comment 6

12 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
Last Resolved: 12 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.