Brian Grinstead [:bgrins] Reporter
	Description • 10 years ago

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).

	Brian Grinstead [:bgrins] Reporter
	Comment 2 • 10 years ago

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/controls@2x.png             (../shared/devtools/images/controls@2x.png)
>  * 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/*?

	Johannes Mittendorfer Assignee
	Comment 3 • 10 years ago

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

	Brian Grinstead [:bgrins] Reporter
	Comment 4 • 10 years ago

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.

	Johannes Mittendorfer Assignee
	Comment 5 • 10 years ago

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

	Brian Grinstead [:bgrins] Reporter
	Comment 6 • 10 years ago

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.

	Johannes Mittendorfer Assignee
	Comment 7 • 10 years ago

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.

	Brian Grinstead [:bgrins] Reporter
	Comment 8 • 10 years ago

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

	Dão Gottwald [:dao]
	Comment 9 • 10 years ago

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

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 10 • 10 years ago

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