Closed
Bug 1210989
Opened 8 years ago
Closed 8 years ago
Rename overlay_send_tab_icon -> shareplane
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox45 fixed)
RESOLVED
FIXED
Firefox 45
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: mcomella, Assigned: alex_johnson, Mentored)
References
Details
(Whiteboard: [lang=xml][good first bug])
Attachments
(4 files, 4 obsolete files)
Comment hidden (typo) |
Reporter | ||
Comment 1•8 years ago
|
||
icon_shareplane [1] and overlay_send_tab_icon have the same content. We should remove the smaller one and resize the larger one to fit in the smaller space. The final icon name should be shareplane.png. [1]: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/drawable-xhdpi/icon_shareplane.png [2]: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/drawable-xhdpi/overlay_send_tab_icon.png
Reporter | ||
Comment 2•8 years ago
|
||
icon_shareplane is unused as of bug 1140048 so instead, we should rename overlay_send_tab_icon to shareplane. To start, set up a build environment - you can see the instructions here: https://wiki.mozilla.org/Mobile/Fennec/Android Then, you'll need to create a patch to upload - see https://wiki.mozilla.org/Mobile/Fennec/Android#Creating_commits_and_submitting_patches If you need any help, you can reply to this bug, or feel free to message me on IRC - my nick is "mcomella" and you can find me in #mobile. If you need IRC setup instructions, see https://wiki.mozilla.org/IRC Thanks and happy coding! ^_^
Depends on: 1140048
Summary: Consolidate share plane icons → Rename overlay_send_tab_icon -> shareplane
Whiteboard: [lang=java][lang=xml] → [lang=xml][good first bug]
Assignee | ||
Comment 3•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=742d40616767
Assignee | ||
Comment 4•8 years ago
|
||
Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella
Attachment #8682734 -
Flags: review?(michael.l.comella)
Assignee | ||
Comment 5•8 years ago
|
||
Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella
Attachment #8682735 -
Flags: review?(michael.l.comella)
Assignee | ||
Comment 6•8 years ago
|
||
Changed the .hgignore file to ignore the gradle local.properties file.
Assignee | ||
Comment 7•8 years ago
|
||
Comment on attachment 8682736 [details]
MozReview Request: Changed the .hgignore file to ignore the gradle local.properties file.
Changed the .hgignore file to ignore the gradle local.properties file.
Attachment #8682736 -
Flags: review?(michael.l.comella)
Reporter | ||
Updated•8 years ago
|
Assignee: nobody → mrjohnsonalex
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•8 years ago
|
Status: NEW → ASSIGNED
Reporter | ||
Comment 8•8 years ago
|
||
Comment on attachment 8682735 [details] MozReview Request: Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella https://reviewboard.mozilla.org/r/24137/#review21623 Can you also rename the xxhdpi version of `overlay_send_tab_icon`? Also can you merge the removal with the addition of the `@drawable/shareplane`? Be sure to use `hg mv` rather than `delete && add` so the repository follows the change history. Regarding, `icon_shareplane`, nice job noticing it's unused! We generally try to put changes like these in their own commit to make them easier to manipulate in the future (e.g. landing it on a different branch without landing other unrelated changes) – can you do that? It's also helpful if they're the initial or post commit so it doesn't interrupt hg range commands, but I won't be that picky this time. :P
Attachment #8682735 -
Flags: review?(michael.l.comella)
Reporter | ||
Comment 9•8 years ago
|
||
Comment on attachment 8682734 [details] MozReview Request: Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella https://reviewboard.mozilla.org/r/24135/#review21625 See my comments on the other commit.
Attachment #8682734 -
Flags: review?(michael.l.comella)
Reporter | ||
Updated•8 years ago
|
Attachment #8682736 -
Flags: review?(michael.l.comella)
Reporter | ||
Comment 10•8 years ago
|
||
Comment on attachment 8682736 [details] MozReview Request: Changed the .hgignore file to ignore the gradle local.properties file. https://reviewboard.mozilla.org/r/24151/#review21627 This should probably be a separate bug because it will require some discussion – why did you decide to ignore this file?
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 11•8 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #10) > Comment on attachment 8682736 [details] > MozReview Request: Changed the .hgignore file to ignore the gradle > local.properties file. > > https://reviewboard.mozilla.org/r/24151/#review21627 > > This should probably be a separate bug because it will require some > discussion – why did you decide to ignore this file? Yeah, this shouldn't be needed. It's a by-product, I think, of using mobile/android/gradle/gradlew -- which mostly works, but isn't supported. Run |mach gradle| instead. Soon, we'll run gradlew in topsrcdir and this confusion will go away.
Assignee | ||
Comment 12•8 years ago
|
||
Comment on attachment 8682734 [details] MozReview Request: Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella
Attachment #8682734 -
Flags: review?(michael.l.comella)
Assignee | ||
Updated•8 years ago
|
Attachment #8682735 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Attachment #8682736 -
Attachment is obsolete: true
Assignee | ||
Comment 13•8 years ago
|
||
Bug 1210989 Removed all instances of the unused file icon_shareplane.png. r?mcomella
Attachment #8682871 -
Flags: review?(michael.l.comella)
Assignee | ||
Comment 14•8 years ago
|
||
Bug 1220145 - DownloadContentService: Check space on disk before downloading content. r?sebastian
Attachment #8682903 -
Flags: review?(s.kaspari)
Reporter | ||
Comment 15•8 years ago
|
||
(In reply to Alex Johnson from comment #14) > Created attachment 8682903 [details] > MozReview Request: Bug 1220145 - DownloadContentService: Check space on disk > before downloading content. r?sebastian The current changeset and its ancestors are pushed to one bug if you specify `hg push review` – you need to explicitly specify a range via `hg push review -r `<first-commit>::<last-commit>` where first commit is the first commit to affect the code base (not the first commit at the top of `hg log`). Note that there is a bug where you can't actually specify one commit (i.e. `hg push review -r <id>` doesn't work) – I usually add a "dummy commit" where I add a file with a little content to the base directory and delete this before pushing so I can push the two commits rather than one. Alternatively, if the commits don't rely on each other, as they don't appear to here, you can use multiple heads – check out the shared commit (which I keep a bookmark for) and make a new commit to create a new head. If you need more help with this, irc would probably be easiest.
Comment 16•8 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #15) > (In reply to Alex Johnson from comment #14) > > Created attachment 8682903 [details] > > MozReview Request: Bug 1220145 - DownloadContentService: Check space on disk > > before downloading content. r?sebastian > > The current changeset and its ancestors are pushed to one bug if you specify > `hg push review` – you need to explicitly specify a range via `hg push > review -r `<first-commit>::<last-commit>` where first commit is the first > commit to affect the code base (not the first commit at the top of `hg > log`). Note that there is a bug where you can't actually specify one commit > (i.e. `hg push review -r <id>` doesn't work) – I usually add a "dummy > commit" where I add a file with a little content to the base directory and > delete this before pushing so I can push the two commits rather than one. mcomella, others: the reviewboard hg plugin adds a non-standard "-c" option to pick exactly one commit, so that hg push review -c ONE_COMMIT does what you expect `hg push review -r ONE_COMMIT` would do.
Reporter | ||
Updated•8 years ago
|
Attachment #8682871 -
Flags: review?(michael.l.comella) → review+
Reporter | ||
Comment 17•8 years ago
|
||
Comment on attachment 8682871 [details] MozReview Request: Bug 1210989 Removed all instances of the unused file icon_shareplane.png. r?mcomella https://reviewboard.mozilla.org/r/24161/#review21697 I actually found this was bug 1216312, but it's inactive so I'm just going to dupe that to this.
Reporter | ||
Comment 19•8 years ago
|
||
Comment on attachment 8682734 [details] MozReview Request: Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella https://reviewboard.mozilla.org/r/24135/#review21699 Looks good!
Attachment #8682734 -
Flags: review?(michael.l.comella) → review+
Reporter | ||
Comment 20•8 years ago
|
||
Alex, at the top right of the reviewboard push, there is a dropdown tab called "Automation" – can you trigger a try build? Only the author can trigger builds, atm. Once that build goes green, add the checkin-needed flag (and obsolete the changes for bug 1220145)!
Flags: needinfo?(mrjohnsonalex)
Assignee | ||
Comment 21•8 years ago
|
||
https://reviewboard.mozilla.org/r/24167/#review21761 Wrong bug!
Assignee | ||
Updated•8 years ago
|
Attachment #8682903 -
Flags: review?(s.kaspari)
Assignee | ||
Comment 22•8 years ago
|
||
Comment on attachment 8682903 [details] MozReview Request: Bug 1220145 - DownloadContentService: Check space on disk before downloading content. r?sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24167/diff/1-2/
Assignee | ||
Updated•8 years ago
|
Attachment #8682903 -
Attachment is obsolete: true
Assignee | ||
Comment 23•8 years ago
|
||
Comment on attachment 8682734 [details] MozReview Request: Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24135/diff/2-3/
Attachment #8682734 -
Attachment description: MozReview Request: Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella → MozReview Request: Bug 1210989 SendTabDeviceListArrayAdapter.java needed to be changed to point to shareplane.png. r?mcomella
Attachment #8682734 -
Flags: review+ → review?(michael.l.comella)
Assignee | ||
Updated•8 years ago
|
Attachment #8682871 -
Attachment is obsolete: true
Reporter | ||
Comment 24•8 years ago
|
||
Comment on attachment 8682734 [details] MozReview Request: Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella https://reviewboard.mozilla.org/r/24135/#review21775 It looks like we lost your previous commits – can you add them back? This commit should probably be squashed in with the other. Let me know if you need any help.
Attachment #8682734 -
Flags: review?(michael.l.comella)
Assignee | ||
Updated•8 years ago
|
Attachment #8682734 -
Attachment description: MozReview Request: Bug 1210989 SendTabDeviceListArrayAdapter.java needed to be changed to point to shareplane.png. r?mcomella → MozReview Request: Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella
Attachment #8682734 -
Flags: review?(michael.l.comella)
Assignee | ||
Comment 25•8 years ago
|
||
Comment on attachment 8682734 [details] MozReview Request: Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24135/diff/3-4/
Assignee | ||
Comment 26•8 years ago
|
||
Bug 1210989 - Removed all the instance of the unused file icon_shareplane.png. r?mcomella
Attachment #8683481 -
Flags: review?(michael.l.comella)
Assignee | ||
Comment 27•8 years ago
|
||
Bug 1210989 SendTabDeviceListArrayAdapter.java needed to be changed to point to shareplane.png. r?mcomella
Attachment #8683482 -
Flags: review?(michael.l.comella)
Reporter | ||
Comment 28•8 years ago
|
||
Comment on attachment 8682734 [details] MozReview Request: Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella https://reviewboard.mozilla.org/r/24135/#review21967
Attachment #8682734 -
Flags: review?(michael.l.comella) → review+
Reporter | ||
Updated•8 years ago
|
Attachment #8683481 -
Flags: review?(michael.l.comella) → review+
Reporter | ||
Comment 29•8 years ago
|
||
Comment on attachment 8683481 [details] MozReview Request: Bug 1210989 - Removed all the instance of the unused file icon_shareplane.png. r?mcomella https://reviewboard.mozilla.org/r/24323/#review21969
Reporter | ||
Comment 30•8 years ago
|
||
Comment on attachment 8683482 [details] MozReview Request: Bug 1210989 SendTabDeviceListArrayAdapter.java needed to be changed to point to shareplane.png. r?mcomella https://reviewboard.mozilla.org/r/24363/#review21971 In general, it'd be better to fold this into the commit where you removed the asset. It's good to make sure each commit stands on its own so when a dev checks out a random commit, they don't get a build that doesn't compile. It can also help you keep track of your own code, and single out your own bugs. However, it's a small change that's unlikely to cause regressions so don't worry about it now. Thanks for your help, Alex! Don't forget to submit this try via reviewboard before adding `checkin-needed`.
Attachment #8683482 -
Flags: review?(michael.l.comella) → review+
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(mrjohnsonalex)
Keywords: checkin-needed
Assignee | ||
Updated•8 years ago
|
Comment 31•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/a4d2428a5016 https://hg.mozilla.org/integration/fx-team/rev/08488a55289e https://hg.mozilla.org/integration/fx-team/rev/6bc04f9eddd6
Keywords: checkin-needed
Comment 32•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a4d2428a5016 https://hg.mozilla.org/mozilla-central/rev/08488a55289e https://hg.mozilla.org/mozilla-central/rev/6bc04f9eddd6
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Reporter | ||
Comment 33•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/ad99e0334c923b1d80036e57190dda69ee9a1096 Bug 1210989 - Backout 3 changesets to backout top level shareplane (bug 1140048).
Reporter | ||
Comment 34•8 years ago
|
||
re backout: see bug 1140048 comment 83. We should rebase and reland this. It will force the patches in bug 1140048 to be rebased so I'll change from depends on to blocking.
Reporter | ||
Comment 35•8 years ago
|
||
Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=ally
Attachment #8689616 -
Flags: review?(ally)
Reporter | ||
Updated•8 years ago
|
Attachment #8689616 -
Flags: review+
Reporter | ||
Comment 36•8 years ago
|
||
Comment on attachment 8689616 [details] MozReview Request: Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=ally https://reviewboard.mozilla.org/r/25675/#review23097 Actually, I'm just going to reland this straight up. (can't seem to clear in RB so I'm going to r+ myself)
Reporter | ||
Comment 37•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/6d37c38e184d91e2fe17a09ccc3354b65ce2f168 Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=me
Comment 38•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6d37c38e184d
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → FIXED
Comment 39•8 years ago
|
||
My dashboard says that I'm needed for review, but this appears to be landed & resolved. Do you still need a review?
Flags: needinfo?(michael.l.comella)
Reporter | ||
Comment 40•8 years ago
|
||
(In reply to Allison Naaktgeboren :ally from comment #39) > My dashboard says that I'm needed for review, but this appears to be landed > & resolved. Do you still need a review? No – I couldn't figure out how to resolve the review on rb so I just resolved it on bz.
Flags: needinfo?(michael.l.comella)
Reporter | ||
Updated•8 years ago
|
Attachment #8689616 -
Flags: review?(ally)
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.