Remove leftover silhouette-40.svg file

RESOLVED FIXED in Firefox 54

Status

()

Firefox
Theme
--
trivial
RESOLVED FIXED
6 months ago
6 months ago

People

(Reporter: florian, Assigned: Vineet Reddy, Mentored, NeedInfo)

Tracking

({good-first-bug})

53 Branch
Firefox 54
good-first-bug
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed)

Details

(URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

6 months ago
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

6 months ago
Is this open?
(Reporter)

Comment 2

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

Yes, you can take it if you want.

Comment 3

6 months ago
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!
(Reporter)

Comment 4

6 months ago
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

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

Comment 6

6 months ago
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!
(Reporter)

Comment 7

6 months ago
(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

6 months ago
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.
(Reporter)

Comment 9

6 months ago
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

6 months ago
Ok cool. Sorry for asking so many questions.

Comment 11

6 months ago
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)

Comment 13

6 months ago
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 hidden (mozreview-request)
(Assignee)

Comment 15

6 months ago
mozreview-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
(Reporter)

Comment 16

6 months ago
mozreview-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/#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+

Comment 17

6 months ago
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

Comment 18

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f73bd485f7d7
Status: NEW → RESOLVED
Last Resolved: 6 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
Assignee: nobody → hotshot2797
You need to log in before you can comment on or make changes to this bug.