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)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bhearsum, Assigned: bhearsum)

References

Details

Attachments

(1 file, 1 obsolete file)

(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. 
Priority: -- → P2
Priority: P2 → P3
Priority: P3 → P2
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-
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+
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
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.