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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 11
Tracking Status
firefox10 - wontfix

People

(Reporter: dao, Assigned: dao)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch patch (obsolete) (deleted) — Splinter Review
E.g. browser/themes/winstripe/browser/browser.css becomes browser/themes/winstripe/browser.css.
Attachment #573347 - Flags: review?(gavin.sharp)
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+
Attached patch patch v2 (deleted) — Splinter Review
Attachment #573347 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 11
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.
(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 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?
Have you verified that this applies cleanly and moves all files on aurora?
verifying. Patch applies cleanly, but I'll check out a build to make sure that everything's where it should be.
verified. Build completed, opened up and ran a full suite of mochi-browser-tests with no errors.
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?
(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+.
(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?
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 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-
VERY WELL! (oops, caps got locked somehow)
(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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: