Closed
Bug 892972
Opened 11 years ago
Closed 10 years ago
make it possible to control whatsnew page showing or not via ship it, per-locale
Categories
(Release Engineering :: Release Automation: Other, defect, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: bhearsum, Assigned: nthomas)
References
Details
(Whiteboard: [Australis:M-])
Attachments
(1 file, 1 obsolete file)
7.08 KB,
patch
|
bhearsum
:
review+
bhearsum
:
checked-in+
|
Details | Diff | Splinter Review |
The whatsnew page is controlled by "actions silent" in the patcher config. We could either add a text field on ship it that lets users add arbitrary actions (because "silent" isn't the only available one), or we could add a checkbox that deals with whatsnew explicitly. The other question is what should be updating the patcher config? We could either add support to patcher-config-bump.pl for actions, or have release runner update the patcher config directly. I'm a bit torn on this part. I think having patcher-config-bump.pl do it makes the most sense, since it already modifies the config in a bunch of ways. If we do that, we need to add support for action modification to it, and figure out how we want to pass this information down the ship it -> buildbot-configs -> buildbot factory -> patcher-config-bump.pl chain.
Assignee | ||
Comment 1•11 years ago
|
||
Is the scope here only a global on/off ? There was a query in email of being able to show whatsnew for only one/some locale(s).
Reporter | ||
Comment 2•11 years ago
|
||
(In reply to Nick Thomas [:nthomas] from comment #1) > Is the scope here only a global on/off ? There was a query in email of being > able to show whatsnew for only one/some locale(s). Huh, that part didn't click with me. As it's scoped now, this would be a global control. A per locale control would require patcher config format changes, I think? And then we'd have to figure out accept that input on Ship It in a reasonable manner.
Comment 3•11 years ago
|
||
Ideally, we would like to turn on /whatsnew per locale and/or have different content per locale, but it isn't a blocker for turning /whatsnew back on in August.
Reporter | ||
Comment 4•11 years ago
|
||
(In reply to Jennifer Bertsch [:jbertsch] from comment #3) > different > content per locale, FYI: This part is 100% handled by webdev, so we won't be addressing that here. > Ideally, we would like to turn on /whatsnew per locale > but it isn't a blocker for turning /whatsnew back on in > August. OK, that's good to know. Given that, I think we should figure out how to do this per-locale as part of this bug. I don't think it makes sense to add a global toggle now just to replace it with a per-locale toggle later. This bug doesn't block turning it on in August, though. We can do that by hand as long as we're informed prior to the release starting.
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
Reporter | ||
Updated•11 years ago
|
OS: Linux → All
Priority: P3 → --
Summary: make it possible to control whatsnew page showing or not via ship it → make it possible to control whatsnew page showing or not via ship it, per-locale
Comment 5•11 years ago
|
||
Tracking for FF25, since we want to do this in the 25 timeframe.
tracking-firefox25:
--- → +
Assignee | ||
Comment 6•11 years ago
|
||
Bug 907404 is going to make actions=silent the in-app default. To get a whatsnew page will need to set this in the appropriate snippets: actions="showURL" openURL="http://blah.com/" It looks from the code at http://mxr.mozilla.org/mozilla-central/source/browser/components/nsBrowserContentHandler.js#568 that the url provided will not be interpolated at runtime, so will have take care of expanding %LOCALE% and %VERSION% in https://www.mozilla.org/%LOCALE%/firefox/%VERSION%/whatsnew/?oldversion=%OLD_VERSION% when generating the snippets. bhearsum and I talked about implementation a bit. He said he could probably do the release-runner side of this mid-september, and I volunteered to take care of buildbot/patcher-config bumper/snippet generation.
Assignee: nobody → nthomas
Priority: -- → P2
Assignee | ||
Comment 7•11 years ago
|
||
(In reply to Nick Thomas [:nthomas] from comment #6) > Bug 907404 is going to make actions=silent the in-app default. To get a > whatsnew page will need to set this in the appropriate snippets: > actions="showURL" openURL="http://blah.com/" Oops, that's the update.xml. The snippets will have actions=showURL openURL=http://blah.com/
Comment 8•11 years ago
|
||
Hi Nick and Ben- I just wanted to check in to see if this fix is still on track for Fx 25 on Oct 29? If yes, we'll ask to proceed with https://bugzilla.mozilla.org/show_bug.cgi?id=927975 Thanks, Jen
Comment 9•11 years ago
|
||
Just emailed Nick to find out whether we need a one-off update for pl, since it doesn't appear this will be completed by next Tuesday.
Reporter | ||
Comment 10•11 years ago
|
||
(In reply to Alex Keybl [:akeybl] from comment #9) > Just emailed Nick to find out whether we need a one-off update for pl, since > it doesn't appear this will be completed by next Tuesday. Yeah, we didn't get to this, unfortunately :(. We'd need it before we start the 25.0 builds anyways, which is today. However, we can set pl to show the whatsnew page by hand. I'll make sure the person taking care of 25.0 is in the loop on what needs to be done. Apologies for not getting to this.
Reporter | ||
Comment 11•11 years ago
|
||
Per comment #7, we need to modify the "actions" section of the snippets and add "openURL". This should do the trick: for snippet in `find Firefox-25.0-build1 -path '*/pl/*'`; do sed -i -e 's/actions=silent/actions=showURL/' $snippet echo "openURL=https://www.mozilla.org/pl/firefox/25.0/whatsnew/?oldversion=%OLD_VERSION%" >> $snippet done
Comment 12•11 years ago
|
||
(In reply to Ben Hearsum [:bhearsum] from comment #10) > (In reply to Alex Keybl [:akeybl] from comment #9) > > Just emailed Nick to find out whether we need a one-off update for pl, since > > it doesn't appear this will be completed by next Tuesday. > > Yeah, we didn't get to this, unfortunately :(. We'd need it before we start > the 25.0 builds anyways, which is today. > > However, we can set pl to show the whatsnew page by hand. I'll make sure the > person taking care of 25.0 is in the loop on what needs to be done. > > Apologies for not getting to this. That's file for now as long as we can get /pl enabled. Abstracting that logic and making it easier to do in future is still the goal. Thanks again.
Updated•11 years ago
|
tracking-firefox25:
+ → ---
Comment 13•11 years ago
|
||
(In reply to Ben Hearsum [:bhearsum] from comment #11) > Per comment #7, we need to modify the "actions" section of the snippets and > add "openURL". This should do the trick: > for snippet in `find Firefox-25.0-build1 -path '*/pl/*'`; do > sed -i -e 's/actions=silent/actions=showURL/' $snippet > echo > "openURL=https://www.mozilla.org/pl/firefox/25.0/whatsnew/ > ?oldversion=%OLD_VERSION%" >> $snippet > done https://wiki.mozilla.org/Releases/Firefox_25.0/BuildNotes
Assignee | ||
Comment 14•10 years ago
|
||
Adds support for an action-locales parameter to the patcher config, which lets you set what locales an action applies to (or all if you leave it out). You get snippet diffs like this: --- reference/aus2/Firefox/26.0/Darwin_x86-gcc3-u-i386-x86_64/20131205075310/en-US/release/complete.txt 2014-03-14 21:43:08.000000000 +1300 +++ aus2/Firefox/26.0/Darwin_x86-gcc3-u-i386-x86_64/20131205075310/en-US/release/complete.txt 2014-03-14 22:42:06.000000000 +1300 @@ -11,2 +11,3 @@ detailsUrl=https://www.mozilla.com/en-US/firefox/28.0/releasenotes/ -actions=silent +actions=showURL +openURL=https://www.mozilla.org/en-US/firefox/28.0/whatsnew/ which is what we wanted. But ... diff -rU1 reference/aus2/Firefox/26.0/Darwin_x86_64-gcc3-u-i386-x86_64/20131205075310/ach/release/complete.txt aus2/Firefox/26.0/Darwin_x86_64-gcc3-u-i386-x86_64/20131205075310/ach/release/complete.txt --- reference/aus2/Firefox/26.0/Darwin_x86_64-gcc3-u-i386-x86_64/20131205075310/ach/release/complete.txt 2014-03-14 21:43:08.000000000 +1300 +++ aus2/Firefox/26.0/Darwin_x86_64-gcc3-u-i386-x86_64/20131205075310/ach/release/complete.txt 2014-03-14 22:42:06.000000000 +1300 @@ -11,2 +11 @@ detailsUrl=https://www.mozilla.com/ach/firefox/28.0/releasenotes/ -actions=silent which we maybe don't. Silent became the default in Firefox 24.0 (via http://hg.mozilla.org/releases/mozilla-release/rev/d3736955cf31), so it might just work fine. Needs testing. Patch needs some tests too.
Assignee | ||
Comment 15•10 years ago
|
||
I took Firefox 23.0, and fed it the current update.xml but with actions="silent" removed. No page was opened after restart. That makes sense because startup.homepage_override_url would be blank then. As the pref goes back at least as far as Firefox 12 (the last update watershed) we're ok to remove silent from our configs and xml.
Assignee | ||
Comment 16•10 years ago
|
||
I added some tests since the WIP. The basic idea here is to read the optional schema 2 args if * the locale is the list actions-locale * or any locale if one isn't given
Attachment #8391684 -
Attachment is obsolete: true
Attachment #8392129 -
Flags: review?(bhearsum)
Assignee | ||
Comment 17•10 years ago
|
||
re testing of snippets, I've took the beta config and truncated the last release to a locale list of 'de en-US ru', then tested * no action-locales --> no change in snippets * action-locales set to 'de en-US', de snippets: -actions=silent +actions=showURL +openURL=https://www.mozilla.org/de/firefox/29.0/whatsnew/ en-US snippets: -actions=silent +actions=showURL +openURL=https://www.mozilla.org/en-US/firefox/29.0/whatsnew/ ru snippets: -actions=silent Also tested for just 'en-US' in action-locales, which works too (de like ru).
Reporter | ||
Comment 18•10 years ago
|
||
Comment on attachment 8392129 [details] [diff] [review] [tools] Add action-locale directive to patcher config Review of attachment 8392129 [details] [diff] [review]: ----------------------------------------------------------------- I noticed that one of the existing tests (and your new one, probably because of copy/paste) wasn't getting run because of a typo "tes" vs. "test". The test fails ('actions' vs. ['actions']), so I'll fix that when I land this. ::: lib/python/release/updates/patcher.py @@ +66,5 @@ > + self['release'][version]['locales']): > + for attr in SCHEMA_2_OPTIONAL_ATTRIBUTES: > + if attr in self['current-update']: > + attrs[attr] = substitutePath(self['current-update'][attr], > + locale=locale) This seems OK, but I think something needs renaming. The patcher config list is called "action-locales", but we've been calling things "attributes" in most other places. I'm not going to block on this, but I'd like to see the naming fixed up before this bug is closed.
Attachment #8392129 -
Flags: review?(bhearsum) → review+
Reporter | ||
Updated•10 years ago
|
Attachment #8392129 -
Flags: checked-in+
Assignee | ||
Comment 19•10 years ago
|
||
Good catch on the test. Do you mean that action-locales isn't a schema attribute like actions/openURL/etc ? I had been treating it like from/to/channel/testchannel - a directive to the snippet generator rather than something that goes in the snippets.
Updated•10 years ago
|
Whiteboard: [Australis:M-]
Reporter | ||
Comment 20•10 years ago
|
||
(In reply to Nick Thomas [:nthomas] from comment #19) > Good catch on the test. Do you mean that action-locales isn't a schema > attribute like actions/openURL/etc ? I had been treating it like > from/to/channel/testchannel - a directive to the snippet generator rather > than something that goes in the snippets. Ah, I see what you mean. Makes sense to me with that in mind. It's in line name-wise with things like "force", too.
Assignee | ||
Comment 21•10 years ago
|
||
Ok, lets close this out.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•