1004111 - Move remaining themes/*/devtools images into themes/shared/devtools/images

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Brian Grinstead [:bgrins]]

We may as well move the remaining icons all at once.

If we use `hg rename browser/themes/linux/devtools/whatever.png browser/themes/shared/devtools/images/whatever.png` it will keep history on them.

Then we can remove the icons from the remaining folders, and update browser/themes/*/jar.mn to replace (devtools/whatever.png) with (../shared/devtools/images/whatever.png).

Comment 1

[Johannes Mittendorfer]

Attachment: moveimages.patch

Comment 2

[Brian Grinstead [:bgrins]]

Comment on attachment 8415989 [details] [diff] [review]
moveimages.patch

Review of attachment 8415989 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/osx/jar.mn
@@ +306,5 @@
>    skin/classic/browser/devtools/filters.svg                 (../shared/devtools/filters.svg)
>    skin/classic/browser/devtools/controls.png                (../shared/devtools/images/controls.png)
>    skin/classic/browser/devtools/[e-mail]             (../shared/devtools/images/[e-mail])
>  * skin/classic/browser/devtools/widgets.css                 (devtools/widgets.css)
> +  skin/classic/browser/devtools/commandline-icon.png        (../shared/devtools/commandline-icon.png)

Shouldn't all of the references in these files be to ../shared/devtools/images/* instead of ../shared/devtools/*?

Comment 3

[Johannes Mittendorfer]

Attachment: moveimagesV2.patch

Of course! I just noticed and was building in the wrong directory before.

Comment 4

[Brian Grinstead [:bgrins]]

Comment on attachment 8416015 [details] [diff] [review]
moveimagesV2.patch

Review of attachment 8416015 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!  Looks like this will get rid of 44 files and ~200K from the tree.  Let's wait for a green try push before marking checkin-needed just to make sure that it builds on each OS: https://tbpl.mozilla.org/?tree=Try&rev=af1a6b5acaa8.

Comment 5

[Johannes Mittendorfer]

Attachment: moveimagesV3.patch

There were some errors in the tests. This should be fixed now. https://tbpl.mozilla.org/?tree=Try&rev=9ed4afe936fd

Comment 6

[Brian Grinstead [:bgrins]]

Comment on attachment 8416383 [details] [diff] [review]
moveimagesV3.patch

Review of attachment 8416383 [details] [diff] [review]:
-----------------------------------------------------------------

There is something odd about the way this patch is formatted - linux/jar.mn and windows/jar.mn seem to show up twice (the second changes appear to be the ones that were made to fix the original try push).  It looks like it works, but it makes it harder to review and figure out what is going on.  Assuming you are using Mercurial Queues and have the item on your queue you should be able `hg qref` then upload the patch .hg/patches/my-item where my-item is the name of the item on the queue.

Comment 7

[Johannes Mittendorfer]

Attachment: moveimagesV4.patch

That is indeed quite strange. I have no idea how this got into the patch. I tried again and this time is looks like beeing ok. This is the exact version I pushed to the try servers before.

Comment 8

[Brian Grinstead [:bgrins]]

Comment on attachment 8416501 [details] [diff] [review]
moveimagesV4.patch

Review of attachment 8416501 [details] [diff] [review]:
-----------------------------------------------------------------

Went ahead and did a fresh push with the latest patch just to be sure, and it is all green: https://tbpl.mozilla.org/?tree=Try&rev=d7822e5cee52.  Going to mark checkin-needed

Comment 9

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/fx-team/rev/479dbf1077ad

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/479dbf1077ad