Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1841262 — Add coenrolling features to NimbusBuilder #2682

Merged

Conversation

jhugman
Copy link
Contributor

@jhugman jhugman commented Jun 30, 2023

Relates to EXP-3623.
Fixes Bug 1841262.

This commit refactors the android-components implementations of NimbusBuilder and Nimbus to accommodate the new getCoenrollingFeatureIds() API.

This API is internal to Application Services, but needs to be passed in via the constructor of the Nimbus object.

This will not build correctly until mozilla/application-services#5697 lands and reaches this repo.

Because this is a breaking change, we've taken the opportunity to do several things here:

  • pulled onFetchCallback and onApplyCallback into AS
  • make getEnrollingFeatureIds() come from the NimbusBuilder
  • move thread creation from the Nimbus object to the NimbusBuilder.

Pull Request checklist

  • Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry or does not need one
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

After merge

  • Breaking Changes: If this is a breaking Android Components change, please push a draft PR on Reference Browser to address the breaking issues.

To download an APK when reviewing a PR (after all CI tasks finished running):

  1. Click on Checks at the top of the PR page.
  2. Click on the firefoxci-taskcluster group on the left to expand all tasks.
  3. Click on the build-apk-{fenix,focus,klar}-debug task you're interested in.
  4. Click on View task in Taskcluster in the new DETAILS section.
  5. The APK links should be on the right side of the screen, named for each CPU architecture.

GitHub Automation

https://bugzilla.mozilla.org/show_bug.cgi?id=1841262

@github-actions github-actions bot added the 🕵️‍♀️ needs review PRs that need to be reviewed label Jun 30, 2023
@jhugman jhugman added do not land PRs that requires coordination before landing and removed 🕵️‍♀️ needs review PRs that need to be reviewed labels Jun 30, 2023
@jeddai
Copy link
Contributor

jeddai commented Jul 5, 2023

I'm hitting significant issues trying to test this and having to reset a lot of my dev box (larger issues than this PR). Not sure if someone else can pick this up to review but if it's passing the builds tomorrow I'll focus on reviewing the code and approve.

@jhugman jhugman force-pushed the jhugman/bug-1841262-add-coenrolling-features-to-nimbus-builder branch from 5a4a644 to 306ef3f Compare July 6, 2023 13:56
This commit refactors the android-components implementations of `NimbusBuilder`
and `Nimbus` to accommodate the new `getCoenrollingFeatureIds()` API.

This API is internal to Application Services, but needs to be passed in
via the constructor of the Nimbus object.
@github-actions github-actions bot added the 🕵️‍♀️ needs review PRs that need to be reviewed label Jul 6, 2023
@jhugman jhugman force-pushed the jhugman/bug-1841262-add-coenrolling-features-to-nimbus-builder branch from 306ef3f to 7205824 Compare July 6, 2023 14:00
@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2023

🚧 Commit message is using the wrong format: Update A-S to 117.20230706050813.

The comment message should look like:

    Bug xxxx - Short description of your change
    Optionally, a longer description of the change.

Copy link
Contributor

@jeddai jeddai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@github-actions github-actions bot added approved PR that has been approved and removed 🕵️‍♀️ needs review PRs that need to be reviewed labels Jul 6, 2023
@jhugman jhugman added 🛬 needs landing PRs that are ready to land and removed do not land PRs that requires coordination before landing labels Jul 6, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2023

🚧 Commit message is using the wrong format: Update A-S to 117.20230706050813.

The comment message should look like:

    Bug xxxx - Short description of your change
    Optionally, a longer description of the change.

@mergify mergify bot merged commit 3560eab into main Jul 6, 2023
298 of 303 checks passed
@bors bors bot deleted the jhugman/bug-1841262-add-coenrolling-features-to-nimbus-builder branch July 6, 2023 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved PR that has been approved 🛬 needs landing PRs that are ready to land
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants