Closed Bug 1210989 Opened 10 years ago Closed 9 years ago

Rename overlay_send_tab_icon -> shareplane

Categories

(Firefox for Android Graveyard :: General, defect)

Product:
Firefox for Android Graveyard
Component:
General
Platform:
All
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox45 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 45
Tracking Flags:
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)

MozReview Request: Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella
40 bytes, text/x-review-board-request
mcomella
: review+
Details
MozReview Request: Bug 1210989 - Removed all the instance of the unused file icon_shareplane.png. r?mcomella
40 bytes, text/x-review-board-request
mcomella
: review+
Details
MozReview Request: Bug 1210989 SendTabDeviceListArrayAdapter.java needed to be changed to point to shareplane.png. r?mcomella
40 bytes, text/x-review-board-request
mcomella
: review+
Details
MozReview Request: Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=ally
40 bytes, text/x-review-board-request
mcomella
: review+
Details
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
Blocks: clean-drawables, fatfennec
Mentor: michael.l.comella
Whiteboard: [lang=java][lang=xml]
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]
Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella
Attachment #8682734 - Flags: review?(michael.l.comella)
Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella
Attachment #8682735 - Flags: review?(michael.l.comella)
Changed the .hgignore file to ignore the gradle local.properties file.
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)
Assignee: nobody → mrjohnsonalex
Status: NEW → ASSIGNED
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)
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)
Attachment #8682736 - Flags: review?(michael.l.comella)
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?
Keywords: checkin-needed
(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.
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)
Attachment #8682735 - Attachment is obsolete: true
Attachment #8682736 - Attachment is obsolete: true
Bug 1210989 Removed all instances of the unused file icon_shareplane.png. r?mcomella
Attachment #8682871 - Flags: review?(michael.l.comella)
Bug 1220145 - DownloadContentService: Check space on disk before downloading content. r?sebastian
Attachment #8682903 - Flags: review?(s.kaspari)
(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.
(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.
Attachment #8682871 - Flags: review?(michael.l.comella) → review+
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.
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+
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)
Attachment #8682903 - Flags: review?(s.kaspari)
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/
Attachment #8682903 - Attachment is obsolete: true
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)
Attachment #8682871 - Attachment is obsolete: true
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)
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)
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/
Bug 1210989 - Removed all the instance of the unused file icon_shareplane.png. r?mcomella
Attachment #8683481 - Flags: review?(michael.l.comella)
Bug 1210989 SendTabDeviceListArrayAdapter.java needed to be changed to point to shareplane.png. r?mcomella
Attachment #8683482 - Flags: review?(michael.l.comella)
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+
Attachment #8683481 - Flags: review?(michael.l.comella) → review+
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
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+
Flags: needinfo?(mrjohnsonalex)
Keywords: checkin-needed
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.
Blocks: 1140048
Status: RESOLVED → REOPENED
status-firefox45: fixed → affected
No longer depends on: 1140048
Resolution: FIXED → ---
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)
https://hg.mozilla.org/mozilla-central/rev/6d37c38e184d
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
status-firefox45: affected → fixed
Resolution: --- → FIXED
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)
(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)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: