1339400 - Remove leftover silhouette-40.svg file

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Florian Quèze [:florian]]

This file was introduced in bug 1123518, and the last reference was removed in bug 1184005.

http://searchfox.org/mozilla-central/search?q=silhouette-40.svg

Comment 1

[jason r]

Is this open?

Comment 2

[Florian Quèze [:florian]]

(In reply to jason r from comment #1)
> Is this open?

Yes, you can take it if you want.

Comment 3

[jason r]

Cool sounds good. Basically just delete those leftover silhoutte.svg? This is my first contribution, so any guidance on how to submit the fix will be helpful as well. Thanks!

Comment 4

[Florian Quèze [:florian]]

Yes, delete the files, and the references in the packaging manifests (jar.mn files).

There's information about how to submit a fix at:

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch

https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Comment 5

[jason r]

Ok sounds good! If I have any other questions I'll be sure to ask. Thanks.

Comment 6

[jason r]

Hey Florian, can I get assigned this bug? I actually deleted the .svgs as well as removed references from the jar.mn files so I will be submitting my patch tomorrow. Thanks!

Comment 7

[Florian Quèze [:florian]]

(In reply to jason r from comment #6)
> Hey Florian, can I get assigned this bug?

I usually assign bugs once there's a patch. But yes, the bug will be assigned to you.

Comment 8

[jason r]

And what kind of tests would I run to test functionality? Since it's not a new feature and a trivial bug, I'm not sure what I should use to test it.

Comment 9

[Florian Quèze [:florian]]

I don't think running any specific test is required here. I would just start the browser and quickly check that it's not completely broken.

Comment 10

[jason r]

Ok cool. Sorry for asking so many questions.

Comment 11

[jason r]

Ok I am trying to push it for review but I am getting an error. 

pushing to https://reviewboard-hg.mozilla.org/autoreview
searching for appropriate review repository
redirecting push to https://reviewboard-hg.mozilla.org/gecko
(ignoring public changeset 698de2db1b16 in review request)
abort: no non-public changesets left to review
(add or change the -r argument to include draft changesets)
How can I fix this?

Comment 12

[Mike Conley (:mconley) (:⚙️)]

Hey jason r, it looks like florian is on holiday until February 27th. So I can try to help you out here.

Have you committed your change with hg? Or do you still see uncommitted changes if you do:

hg diff

?

Comment 13

[jason r]

Hey Mike thanks for responding! I actually just switched to Fedora from Ubuntu so I am rebuilding. I will try again in about an hour and let you know my result. However, if I did not commit my changes using hg before I tried to submit it for review, then I would imagine that would be the cause of the error. I will make sure to commit and check the diff before trying again. The commit system here seems a little strange to me as some things I've read say that I can upload a diff file to BugZilla as an attachment or do it using mercurial. Is mercurial the preferred way to submit code for review?

Comment 14

[Vineet Reddy]

Attachment: Bug 1339400 - Removed the leftover silhouette-40.svg files. Removed the instances of the svg file in the respective jar.mn. RM'd the files to make sure mercurial stops tracking the deleted files now.

Review commit: https://reviewboard.mozilla.org/r/116520/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/116520/

Comment 15

[Vineet Reddy]

Comment on attachment 8842756 [details]
Bug 1339400 - Removed the leftover silhouette-40.svg files. Removed the instances of the svg file in the respective jar.mn. RM'd the files to make sure mercurial stops tracking the deleted files now.

https://reviewboard.mozilla.org/r/116520/#review118180

Comment 16

[Florian Quèze [:florian]]

Comment on attachment 8842756 [details]
Bug 1339400 - Removed the leftover silhouette-40.svg files. Removed the instances of the svg file in the respective jar.mn. RM'd the files to make sure mercurial stops tracking the deleted files now.

https://reviewboard.mozilla.org/r/116520/#review118248

Hi Vineet,

Thanks for the patch. It looks good to me so I'm marking it r+, and will land it shortly.

You have probably seen that jason was already trying to work on this bug. In the future, before taking a bug on which someone has started, it would be nice to ask them how they feel about it. There are several other bugs that are similar to this one in the dependencies of bug 1316187 (https://bugzilla.mozilla.org/showdependencytree.cgi?id=1316187&hide_resolved=1), and I filed some more today, so it shouldn't be difficult to find another one where nobody has started yet (and feel free to pick another one from this list for your next bug if you would like to continue removing files that we ship but don't use anywhere).

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f73bd485f7d7
Removed the leftover silhouette-40.svg files. Removed the instances of the svg file in the respective jar.mn. RM'd the files to make sure mercurial stops tracking the deleted files now. r=florian

Comment 18

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

https://hg.mozilla.org/mozilla-central/rev/f73bd485f7d7