The default bug view has changed. See this FAQ.

Get rid of "browser" sub-folder in browser/themes/*stripe/

RESOLVED FIXED in Firefox 11

Status

()

Firefox
Theme
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dao, Assigned: dao)

Tracking

Trunk
Firefox 11
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox10- wontfix)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 573347 [details] [diff] [review]
patch

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+
(Assignee)

Comment 2

5 years ago
Created attachment 573435 [details] [diff] [review]
patch v2
Attachment #573347 - Attachment is obsolete: true
(Assignee)

Comment 3

5 years ago
https://hg.mozilla.org/mozilla-central/rev/a41fa578cfc1
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 11

Comment 4

5 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.
(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?
tracking-firefox10: --- → ?
Blocks: 700770
(Assignee)

Comment 7

5 years ago
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.
(Assignee)

Comment 10

5 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?
(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

5 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
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.
Blocks: 700333
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)
status-firefox10: --- → wontfix
(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

5 years ago
tracking-firefox10: ? → -
You need to log in before you can comment on or make changes to this bug.