Closed Bug 1339400 Opened 7 years ago Closed 7 years ago

Remove leftover silhouette-40.svg file

Categories

(Firefox :: Theme, defect)

53 Branch
defect
Not set
trivial

Tracking

()

RESOLVED FIXED
Firefox 54
Tracking Status
firefox54 --- fixed

People

(Reporter: florian, Assigned: hotshot2797, Mentored, NeedInfo)

References

()

Details

(Keywords: good-first-bug)

Attachments

(1 file)

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
Is this open?
(In reply to jason r from comment #1)
> Is this open?

Yes, you can take it if you want.
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!
Ok sounds good! If I have any other questions I'll be sure to ask. Thanks.
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!
(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.
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.
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.
Ok cool. Sorry for asking so many questions.
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?
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

?
Flags: needinfo?(renaudjj)
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 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 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).
Attachment #8842756 - Flags: review+
Pushed by florian@queze.net:
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
https://hg.mozilla.org/mozilla-central/rev/f73bd485f7d7
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
You need to log in before you can comment on or make changes to this bug.