Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

For #5200: Update build docs to reflect new variants #5220

Merged
merged 3 commits into from
Sep 11, 2019

Conversation

colintheshots
Copy link
Contributor

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
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

After merge

  • Milestone: Make sure issues finished by this pull request are added to the milestone of the version currently in development.

@colintheshots colintheshots requested review from a team as code owners September 11, 2019 17:19
@codecov-io
Copy link

codecov-io commented Sep 11, 2019

Codecov Report

Merging #5220 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #5220   +/-   ##
=========================================
  Coverage     11.55%   11.55%           
  Complexity      239      239           
=========================================
  Files           244      244           
  Lines          9954     9954           
  Branches       1447     1447           
=========================================
  Hits           1150     1150           
  Misses         8733     8733           
  Partials         71       71

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bafbea1...9cb5e06. Read the comment docs.

README.md Outdated
**armDebug** for ARM
**x86Debug** for X86
**geckoBetaDebug** for the beta version of the Gecko engine
**geckoNightlyDebug** for the nightly version of the Gecko engine
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd also add "recommended" on the geckoBetaDebug line.

Optional: could we also add a small section explaining what the other variants are? Or describe how this naming is made (e.g. geckoVersionFenixVersion).

README.md Outdated
@@ -58,12 +59,14 @@ Before you attempt to make a contribution please read the [Community Participati
2. Import the project into Android Studio **or** build on the command line:
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated: Bold the word "import"

@colintheshots colintheshots force-pushed the updateDocs branch 4 times, most recently from fa4a39a to b49e09d Compare September 11, 2019 18:42
Copy link
Contributor

@liuche liuche left a comment

Choose a reason for hiding this comment

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

thanks @colintheshots ! these explanations are great.
Also, in the future when people change/add build variants, we should remember ask them to update the README too.

README.md Outdated
**fenixNightlyLegacy** is a release build with release signing which uses the org.mozilla.fenix production app id, which we're trying to phase out
**fenixBeta** is a release build with beta signing which uses the org.mozilla.fenix.beta app id for beta releases to Google Play
**fenixProduction** is a release build with release signing which uses the org.mozilla.fenix app id for production relases to Google Play
**fennecProduction** is an experimental build with release signing which uses the org.mozilla.firefox app id and supports upgrading the older Firefox
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should say not recommended or mention it causes data loss

Copy link
Contributor

Choose a reason for hiding this comment

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

People who build fennecProduction won't have access to Mozilla's signing keys so the would not be able to pave over their Fennec release install.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right. It will only work on top of their own build of Fennec, not ours.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add the data loss warning, since people have been messing with it on Reddit.

Copy link
Contributor

@liuche liuche left a comment

Choose a reason for hiding this comment

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

r+, no need to send back.

3. Make sure to select the correct build variant in Android Studio:
**armDebug** for ARM
**x86Debug** for X86
3. Make sure to select the correct build variant in Android Studio. See the next section.
Copy link
Contributor

Choose a reason for hiding this comment

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

I might also suggest adding the recommended flavor in this line too (if people don't want to read the next section).
build variant in Android Studio (recommended: geckoBetaDebug).

@colintheshots colintheshots merged commit bfee95b into mozilla-mobile:master Sep 11, 2019
@colintheshots colintheshots deleted the updateDocs branch September 11, 2019 19:53
@jyeontaek jyeontaek modified the milestone: v2.0 Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants