Remove unused mobile/android config variables

RESOLVED FIXED in Firefox 68

Status

()

defect
RESOLVED FIXED
11 months ago
29 days ago

People

(Reporter: sysrqb, Assigned: sysrqb)

Tracking

Trunk
Firefox 68
Points:
---

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

47 bytes, text/x-phabricator-request
Details | Review
Assignee

Description

11 months ago
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 hidden (obsolete)
Comment hidden (obsolete)
Comment hidden (obsolete)
Status: RESOLVED → UNCONFIRMED
Resolution: INACTIVE → ---
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: good-first-bug

Comment 4

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

Comment 5

9 months ago
Posted patch unusedVars.diff (obsolete) — Splinter Review
Attachment #9009725 - Flags: review+
Attachment #9009725 - Flags: feedback+

Comment 6

9 months ago
Can someone please review the patch I uploaded?
Flags: needinfo?(matthew.finkel)

Comment 7

3 months ago

Id like to contribute to this bug?

Sam

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?

Flags: needinfo?(matthew.finkel) → needinfo?(nalexander)

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

Flags: needinfo?(nalexander)

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?

Flags: needinfo?(korina.houghtaling)
Assignee

Comment 12

Last month
Assignee

Comment 14

Last month

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

Comment 15

Last month

Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e7cfd61c3840
Delete unused confvars entries on Android r=nalexander

Keywords: checkin-needed

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

Flags: needinfo?(korina.houghtaling)

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

Summary: Unused Config Variables → Remove unused mobile/android config variables
Attachment #9009725 - Attachment is obsolete: true
Assignee

Comment 18

Last month

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:
Attachment #9065368 - Flags: approval-mozilla-beta?

Comment 19

Last month
bugherder
Status: NEW → RESOLVED
Closed: 11 months agoLast month
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
Assignee: nobody → matthew.finkel

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

beta is now 68

Attachment #9065368 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
You need to log in before you can comment on or make changes to this bug.