1478438 - Remove unused mobile/android config variables

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Matthew Finkel [:sysrqb]]

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20100101

Steps to reproduce:

There are unused/dead variable in `mobile/android/confvars.sh`. Specifically, I don't see MOZ_XULRUNNER and MOZ_CAPTURE used anywhere in the codebase.

https://dxr.mozilla.org/mozilla-central/search?q=MOZ_XULRUNNER
https://dxr.mozilla.org/mozilla-central/search?q=MOZ_CAPTURE



Expected results:

Can these be deleted?

Comment 1

[Stefan Deiac]

This issue seems to be duplicate of bug 1440661. I will mark this as duplicate of other, please follow the discussion there, thanks.

Comment 2

[Stefan Deiac]

I apologies for mistake. It was a wrong issue reply. Please don't consider the previous comment. Next time I will be more careful.
Thanks.

Comment 3

[Firefox Bug Husbandry Bot]

Closing per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Contact :susheel if you think this bug should be re-opened

Comment 4

[Korina]

Hi! I've never contributed to the open-source community. Can I be assigned this bug?

Comment 5

[Korina]

Attachment: unusedVars.diff

Comment 6

[Korina]

Can someone please review the patch I uploaded?

Comment 7

[samueljselvarajah]

Id like to contribute to this bug?

Sam

Comment 8

[Kevin Brosnan [Ex-Mozilla]]

This bug already has a patch. It is not a good one to take. The needinfo that Korina left went to the wrong person so this got missed.

Nick is it possible to review this patch in its current state or does everything go through Phabricator these days?

Comment 9

[Nick Alexander :nalexander [he/him]]

(In reply to Kevin Brosnan [:kbrosnan] from comment #8)

This bug already has a patch. It is not a good one to take. The needinfo that Korina left went to the wrong person so this got missed.

Nick is it possible to review this patch in its current state or does everything go through Phabricator these days?

Yes. We should also remove the following lines: https://searchfox.org/mozilla-central/rev/f1c7ba91fad60bfea184006f3728dd6ac48c8e56/old-configure.in#1730-1733 and https://searchfox.org/mozilla-central/rev/f1c7ba91fad60bfea184006f3728dd6ac48c8e56/old-configure.in#1742.

Everything does go through Phab these days, which raises the barrier to entry.

samueljselvarajah, if you would like to make these additional changes, please do!

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:korina.houghtaling, could you have a look please?

Comment 11

[Matthew Finkel [:sysrqb]]

Attachment: Bug 1478438 - Delete unused confvars entries on Android

Comment 12

[Matthew Finkel [:sysrqb]]

I pushed a patch for review - https://phabricator.services.mozilla.com/D31421

Comment 13

[Matthew Finkel [:sysrqb]]

I have a try build that looks green - https://treeherder.mozilla.org/#/jobs?repo=try&revision=9a4ee5d70ea23d8edd7d99a82aeae9687a866584

Comment 14

[Matthew Finkel [:sysrqb]]

The patch ready to land, but it is based on mozilla-beta. I probably did this backwards. Can this be uplifted?

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e7cfd61c3840
Delete unused confvars entries on Android r=nalexander

Comment 16

[Nick Alexander :nalexander [he/him]]

(In reply to Matthew Finkel [:sysrqb] from comment #14)

The patch ready to land, but it is based on mozilla-beta. I probably did this backwards. Can this be uplifted?

Heh -- I didn't even notice that it was against beta :) I landed it using Lando (in Phabricator), so we're good. Thanks, Matthew!

Comment 17

[Nick Alexander :nalexander [he/him]]

(In reply to Matthew Finkel [:sysrqb] from comment #14)

The patch ready to land, but it is based on mozilla-beta. I probably did this backwards. Can this be uplifted?

relman: this patch is essentially no risk; it's a build time change (a tiny one) that should only result in broken builds. I'll request approval.

Comment 18

[Matthew Finkel [:sysrqb]]

Comment on attachment 9065368 [details]
Bug 1478438 - Delete unused confvars entries on Android

Beta/Release Uplift Approval Request

• User impact if declined: None. It's only deleting variables that aren't used.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Not risky, but uplifting a tor browser patch.
• String changes made/needed:

Comment 19

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/e7cfd61c3840

Comment 20

[Julien Cristau [:jcristau]]

Comment on attachment 9065368 [details]
Bug 1478438 - Delete unused confvars entries on Android

beta is now 68