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

[Do not merge yet] Update documentation for feature flags and feature launch process. #5390

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

seanlip
Copy link
Member

@seanlip seanlip commented Apr 18, 2024

Explanation

This PR updates the wiki based on discussions with @BenHenning about the feature launch process. In writing this up, I realized that several things need to be done/clarified:

  • Web needs to specify a required min-Android-app-version for prod-stage Android feature flags (i.e. beta and GA).
  • Do flags have a dev/test/prod stage on Android as well as Web? If so, how will the two be kept in sync -- does the developer need to do this manually? What if they are not in sync? This needs to be documented here.
  • We need to set up the Android feature testing team so that the form mentioned in step 3 of the pre-launch testing stage instructions actually goes somewhere.

Nevertheless I would like to get a preliminary review of this PR so it's clear what else needs to be addressed before this can be merged (and which of the above is fine to address post-merge).

Essential Checklist

  • The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • Any changes to scripts/assets files have their rationale included in the PR explanation.
  • The PR follows the style guide.
  • The PR does not contain any unnecessary code changes from Android Studio (reference).
  • The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
  • The PR is assigned to the appropriate reviewers (reference).

Copy link

oppiabot bot commented Apr 25, 2024

Hi @seanlip, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Apr 25, 2024
@adhiamboperes adhiamboperes removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Apr 29, 2024
Copy link

oppiabot bot commented May 6, 2024

Hi @seanlip, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label May 6, 2024
@adhiamboperes adhiamboperes removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label May 8, 2024
Copy link
Collaborator

@adhiamboperes adhiamboperes left a comment

Choose a reason for hiding this comment

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

Thanks @seanlip! I have added thoughts inline.

Additionally,

Web needs to specify a required min-Android-app-version for prod-stage Android feature flags (i.e. beta and GA).
Can be done post merge of this PR if no pending release is reliant on this approach.

Do flags have a dev/test/prod stage on Android as well as Web? If so, how will the two be kept in sync -- does the developer need to do this manually? What if they are not in sync? This needs to be documented here.
No, we do not have stages for the flags on Android.


### Development Stage

1. First, create a PR on Web (following [these instructions](https://github.com/oppia/oppia/wiki/Launching-new-features#follow-the-steps-below-to-add-a-new-feature-flag)) that introduces the feature flag, so that it can be used by Android. The name of the feature flag must start with `android_` and the feature flag should be in DEV stage.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unrelated to this change, but we haven't raised any PRs on web for our new feature flags(AFAIK).


1. First, create a PR on Web (following [these instructions](https://github.com/oppia/oppia/wiki/Launching-new-features#follow-the-steps-below-to-add-a-new-feature-flag)) that introduces the feature flag, so that it can be used by Android. The name of the feature flag must start with `android_` and the feature flag should be in DEV stage.

1. While waiting for that PR to be merged, create a PR on Android following the instructions in [Platform Parameters & Feature Flags](https://github.com/oppia/oppia-android/wiki/Platform-Parameters-&-Feature-Flags) that introduces your feature and adds a new feature flag that is meant to be used with this new feature. The feature flag should be placed in the DEV stage and disabled by default, so that it cannot accidentally be turned on in environments for which it is not yet ready (like the alpha/beta/GA release channels). Every single user-facing aspect of the feature -- whether frontend or backend -- must be gated behind the feature flag (both in the first PR, and all the following ones as well). This is to ensure that the feature is not visible to the user until it is fully implemented and ready for production.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The feature flag should be placed in the DEV stage and disabled by default

We don't have actual classifications for DEV stage on android, but we do disable feature flags by default.


1. If the testing results are positive and the feature is approved by the QA team (i.e. it’s been fully tested, and all issues filed have been fixed + verified), your feature is ready for launch! Create a PR on the Web codebase that moves the feature flag to the PROD stage, allowing it to be enabled/disabled in production and have its min-version set (by the release coordinator(s)). **NOTE: When opening this PR, include a link to the testing doc or other proof that the feature has been approved for release.** While this PR is open, confirm (through discussion) with the Android QA team that the new CUJs for this feature have been added to the overall CUJs used for testing Android releases in general.

- Once this PR is merged, send a ["feature-flag flip request"](https://forms.gle/rUJaHJSpRGemtGDp6) to the release coordinators to turn on the feature in production by adding a rule in the /release-coordinator page.
Copy link
Collaborator

Choose a reason for hiding this comment

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

When I initially read this, I was confused by

by adding a rule in the /release-coordinator page.

I thought it meant, "I, the feature owner, would need to do that".

I think ommiting that sentence fragment will remove confusion but still convey the message.


We need to do some cleanup after the feature is successfully running in production:

1. Once the feature is confirmed to be functioning as intended in production (at least 10% of the 7DA user base is on a version of the app that has the feature enabled) by the product team, please do the following, in order:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it is worth clarifying how to get to this point, e.g. check in 4 weeks after the release(or is the 7 days the actual time after release to check in?), or otherwise file a tracking issue(Which we currently do but can fall through the cracks with no specific follow up guidelines. Especially since the clean up is not typically done by the original feature owner).

Copy link
Collaborator

Choose a reason for hiding this comment

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

We could have a new issue template specifically for Post-launch cleanup, that ensures all the steps needed for clean up are captured.

* [Working on UI](https://github.com/oppia/oppia-android/wiki/Working-on-UI)
* [Writing Design Docs](https://github.com/oppia/oppia-android/wiki/Writing-design-docs)
* **[Guidelines for launching new features](https://github.com/oppia/oppia-android/wiki/Guidelines-for-launching-new-features)**
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Formatting. This item is bold and not consistent with the others in the same hierachy.

1. Create the Constants
- If the Platform Parameter you intend to create is related to a particular feature, so first check that do there exist a file in the `utility\src\main\java\org\oppia\android\util\platformparameter` which contains other Platform Parameters related to the same feature. If there is no such then create a new Kotlin file along with its name corresponding to the feature.
- New feature flags should go in [FeatureFlagConstants.kt](https://github.com/oppia/oppia-android/blob/develop/utility/src/main/java/org/oppia/android/util/platformparameter/FeatureFlagConstants.kt).
- **Note:** Previously, platform parameters and feature flags were mixed together in `utility/src/main/java/org/oppia/android/util/platformparameter`. We are now planning to handle feature flags and platform parameters separately.
- After searching/making a "constants" file related to a feature, we need to define three things:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe replace with:

In the PlatformParameterConstants.kt or FeatureFlagConstants.kt file as appropriate,

Since we do not create any new files besides the two.

Copy link

oppiabot bot commented May 21, 2024

Hi @seanlip, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label May 21, 2024
@adhiamboperes adhiamboperes removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants