Closed
Bug 701212
Opened 13 years ago
Closed 13 years ago
Get rid of "browser" sub-folder in browser/themes/*stripe/
Categories
(Firefox :: Theme, defect)
Firefox
Theme
Tracking
()
RESOLVED
FIXED
Firefox 11
People
(Reporter: dao, Assigned: dao)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
johnath
:
approval-mozilla-aurora-
|
Details | Diff | Splinter Review |
E.g. browser/themes/winstripe/browser/browser.css becomes browser/themes/winstripe/browser.css.
Attachment #573347 -
Flags: review?(gavin.sharp)
Comment 1•13 years ago
|
||
Comment on attachment 573347 [details] [diff] [review]
patch
I think you should leave the "communicator" directories alone - we should probably just get rid of those entirely (if not now then at some point in the future), and merging them in with the other useful stuff makes that easier to forget about. I suppose you could alternately consolidate it and move it up to just be directly in browser/themes.
r=me with that.
Attachment #573347 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 2•13 years ago
|
||
Attachment #573347 -
Attachment is obsolete: true
Assignee | ||
Comment 3•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 11
Comment 4•13 years ago
|
||
When configure creates the Makefile's, I see the following now:
creating browser/installer/windows/Makefile
creating browser/locales/Makefile
creating browser/themes/Makefile
creating browser/themes/pinstripe/browser/Makefile
can't read c:/dev/jenkins/mozilla-central-git/browser/themes/pinstripe/browser/Makefile.in: No such file or directory
creating browser/themes/pinstripe/communicator/Makefile
creating browser/themes/pinstripe/Makefile
creating browser/themes/winstripe/browser/Makefile
can't read c:/dev/jenkins/mozilla-central-git/browser/themes/winstripe/browser/Makefile.in: No such file or directory
creating browser/themes/winstripe/communicator/Makefile
creating browser/themes/winstripe/Makefile
creating browser/branding/nightly/Makefile
creating browser/branding/nightly/content/Makefile
creating browser/branding/nightly/locales/Makefile
You can see this on a Linux build machine: https://tbpl.mozilla.org/php/getParsedLog.php?id=7339477&tree=Firefox&full=1
I'm not sure if the build is now missing Makefiles or what. But, some references definitely need cleaned up.
Comment 5•13 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #4)
> I'm not sure if the build is now missing Makefiles or what. But, some
> references definitely need cleaned up.
The |browser/themes/winstripe/browser/Makefile| entries need to be removed from browser/makefiles.sh (the *makefiles.sh files are for speeding up srcdir -> objdir makefile file creation only, so the warnings aren't dangerous).
I'm doing an overhaul of all of the *makefiles.sh scripts in bug 696498, so will clean this up there by the middle of next week, if someone else hasn't done so by then. I'll also be posting to dev.platform asking people to remember to remove and most importantly add entries when touching makefiles, since bug 696498 is having to deal with ~200 makefiles (out of 1100 in the tree) missing from that list, so it would be good for it to stay as up to date as long as possible.
Comment 6•13 years ago
|
||
Comment on attachment 573435 [details] [diff] [review]
patch v2
would like to land this in aurora as it will make subsequent style changes easier. Low risk, simple file moves.
Attachment #573435 -
Flags: approval-mozilla-aurora?
Updated•13 years ago
|
tracking-firefox10:
--- → ?
Assignee | ||
Comment 7•13 years ago
|
||
Have you verified that this applies cleanly and moves all files on aurora?
Comment 8•13 years ago
|
||
verifying. Patch applies cleanly, but I'll check out a build to make sure that everything's where it should be.
Comment 9•13 years ago
|
||
verified. Build completed, opened up and ran a full suite of mochi-browser-tests with no errors.
Assignee | ||
Comment 10•13 years ago
|
||
browser-chrome tests don't seem interesting here. Also, the patch applies to all three themes separately. When you apply it, are the three browser sub folders gone?
Comment 11•13 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #10)
> browser-chrome tests don't seem interesting here.
I would expect some of our UI tests to fail if there was broken styling. Seemed a possible way to check for brokenness.
> Also, the patch applies to
> all three themes separately. When you apply it, are the three browser sub
> folders gone?
yes. R+.
Comment 12•13 years ago
|
||
(In reply to Rob Campbell [:rc] (robcee) from comment #6)
> Comment on attachment 573435 [details] [diff] [review] [diff] [details] [review]
> patch v2
>
> would like to land this in aurora as it will make subsequent style changes
> easier. Low risk, simple file moves.
[triage comment]
Easier for? We generally don't expect to take these type of fixes on Beta. Are there still outstanding issues on Aurora that this would help with?
Keywords: #relman/triage/needs-info
Comment 13•13 years ago
|
||
Easier for the people landing style-related changes to Aurora, i.e., (in this case), me.
This helps with organization and maintaining consistency between the nightly and aurora branches. In the future, it will help with landing fixes on Beta, should we need to.
Comment 14•13 years ago
|
||
Comment on attachment 573435 [details] [diff] [review]
patch v2
We discussed this in triage today and felt that the maintenance cost of prospective future changes to aurora was not sufficient reason to take an organizational cleanup patch on a branch that should be only for security/stability/feature backouts.
We are known to be pretty damned risk averse, so please re-nom if you think we're underestimating the degree of pain caused by leaving this directory move to ride the trains.
Attachment #573435 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Comment 15•13 years ago
|
||
VERY WELL! (oops, caps got locked somehow)
Updated•13 years ago
|
status-firefox10:
--- → wontfix
Comment 16•13 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #4)
> I'm not sure if the build is now missing Makefiles or what. But, some
> references definitely need cleaned up.
Now done in: https://hg.mozilla.org/integration/mozilla-inbound/rev/79f0181b02f8
Updated•13 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•