1210989 - Rename overlay_send_tab_icon -> shareplane

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

icon_shareplane [1] and overlay_send_tab_icon have the same content. We should remove the smaller one and resize the larger one to .

The final 

[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

Comment 1

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

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

Comment 2

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

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! ^_^

Comment 3

[Alex Johnson (:alex_johnson)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=742d40616767

Comment 4

[Alex Johnson (:alex_johnson)]

Attachment: MozReview Request: Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella

Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella

Comment 5

[Alex Johnson (:alex_johnson)]

Attachment: MozReview Request: Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella

Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella

Comment 6

[Alex Johnson (:alex_johnson)]

Attachment: MozReview Request: Changed the .hgignore file to ignore the gradle local.properties file.

Changed the .hgignore file to ignore the gradle local.properties file.

Comment 7

[Alex Johnson (:alex_johnson)]

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.

Comment 8

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

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

Comment 9

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

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.

Comment 10

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

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?

Comment 11

[Nick Alexander :nalexander [he/him]]

(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 12

[Alex Johnson (:alex_johnson)]

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

Comment 13

[Alex Johnson (:alex_johnson)]

Attachment: MozReview Request: Bug 1210989 Removed all instances of the unused file icon_shareplane.png. r?mcomella

Bug 1210989 Removed all instances of the unused file icon_shareplane.png. r?mcomella

Comment 14

[Alex Johnson (:alex_johnson)]

Attachment: MozReview Request: Bug 1220145 - DownloadContentService: Check space on disk before downloading content. r?sebastian

Bug 1220145 - DownloadContentService: Check space on disk before downloading content. r?sebastian

Comment 15

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

(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

[Nick Alexander :nalexander [he/him]]

(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.

Comment 17

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

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 19

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

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!

Comment 20

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

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)!

Comment 21

[Alex Johnson (:alex_johnson)]

https://reviewboard.mozilla.org/r/24167/#review21761

Wrong bug!

Comment 22

[Alex Johnson (:alex_johnson)]

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/

Comment 23

[Alex Johnson (:alex_johnson)]

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/

Comment 24

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

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.

Comment 25

[Alex Johnson (:alex_johnson)]

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/

Comment 26

[Alex Johnson (:alex_johnson)]

Attachment: MozReview Request: Bug 1210989 - Removed all the instance of the unused file icon_shareplane.png. r?mcomella

Bug 1210989 - Removed all the instance of the unused file icon_shareplane.png. r?mcomella

Comment 27

[Alex Johnson (:alex_johnson)]

Attachment: MozReview Request: Bug 1210989 SendTabDeviceListArrayAdapter.java needed to be changed to point to shareplane.png. r?mcomella

Bug 1210989 SendTabDeviceListArrayAdapter.java needed to be changed to point to shareplane.png. r?mcomella

Comment 28

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

Comment on attachment 8682734 [details]
MozReview Request: Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=mcomella

https://reviewboard.mozilla.org/r/24135/#review21967

Comment 29

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

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 30

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

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`.

Comment 31

[Pulsebot]

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

Comment 32

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

https://hg.mozilla.org/mozilla-central/rev/a4d2428a5016
https://hg.mozilla.org/mozilla-central/rev/08488a55289e
https://hg.mozilla.org/mozilla-central/rev/6bc04f9eddd6

Comment 33

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

https://hg.mozilla.org/integration/fx-team/rev/ad99e0334c923b1d80036e57190dda69ee9a1096
Bug 1210989 - Backout 3 changesets to backout top level shareplane (bug 1140048).

Comment 34

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

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.

Comment 35

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

Attachment: MozReview Request: Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=ally

Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=ally

Comment 36

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

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)

Comment 37

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

https://hg.mozilla.org/integration/fx-team/rev/6d37c38e184d91e2fe17a09ccc3354b65ce2f168
Bug 1210989 - Rename overlay_send_tab_icon -> shareplane. r=me

Comment 38

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6d37c38e184d

Comment 39

[Allison Naaktgeboren :ally]

My dashboard says that I'm needed for review, but this appears to be landed & resolved. Do you still need a review?

Comment 40

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

(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.