Closed Bug 1123377 Opened 5 years ago Closed 5 years ago

Create build flag for reading list service

Categories

(Firefox Build System :: Android Studio and Gradle Integration, defect)

All
Android
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla38

People

(Reporter: rnewman, Assigned: rnewman)

References

Details

Attachments

(1 file, 1 obsolete file)

This will control:

* FxA setup
* Settings
* Doorhangers
* A SyncAdapter
* And many, many more, all at a low, low price.
Blocks: 1123388
Blocks: 1123389
Defaults to off. Doesn't have any permissions/services/etc. yet, so just the stub of a flag and AppConstants.
Attachment #8551385 - Flags: review?(nalexander)
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Comment on attachment 8551385 [details] [diff] [review]
Create build flag for reading list service on Android. v1

Review of attachment 8551385 [details] [diff] [review]:
-----------------------------------------------------------------

Let's add the relevant AC_SUBST to configure.in, and add --enable-reading-list or whatever.  Desktop will want this and it's best to control features in mozconfig rather than confvars.  I think m/a/b/moz.build will need to include the new flag in DEFINES; I expect this doesn't actually pick up the flag right now.

Unless there's a reason to do this in confvars.sh only?
Attachment #8551385 - Flags: review?(nalexander) → review-
The reason I picked the confvars approach rather than a --flag:

* This isn't an external build flag that we expect people to change; this is control for when an in-development feature rides the trains.

* I explicitly don't want this interfering with desktop work, so I don't want reuse there. These two projects don't share anything.

I probably still need something in configure.in, though.
Note that exposing a --flag is orthogonal to enabling in mozconfig or confvars.sh. You can very well enable something without a --flag in a mozconfig.

Now, the patch as it is attached is not going to do anything whether things are added to mozconfig or confvars.sh, because it doesn't do anything explicit with the variable in configure.in. It would need to be AC_SUBSTed and tested in a moz.build to add to DEFINES, or AC_DEFINEd.
Now with 500% more cargo-culting.
Attachment #8552208 - Flags: review?(nalexander)
Attachment #8551385 - Attachment is obsolete: true
Comment on attachment 8552208 [details] [diff] [review]
Create build flag for reading list service on Android. v2

Review of attachment 8552208 [details] [diff] [review]:
-----------------------------------------------------------------

Sure.
Attachment #8552208 - Flags: review?(nalexander) → review+
https://hg.mozilla.org/mozilla-central/rev/090ab7d32dcc
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
Blocks: 1131421
Depends on: 1149226
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 38 → mozilla38
You need to log in before you can comment on or make changes to this bug.