From 64003b80d30019e8d1f0ca26e1adbfaa4a2b0bce Mon Sep 17 00:00:00 2001 From: Sean Lip Date: Sun, 17 Nov 2024 23:32:44 +0800 Subject: [PATCH] Clean up onboarding and launch instructions. (#416) --- Contributing-code-to-Oppia.md | 34 +++++++++++++++++-------- LaCE-onboarding-guide.md | 4 +++ Launching-new-features.md | 48 ++++++++++++++++++++--------------- 3 files changed, 55 insertions(+), 31 deletions(-) diff --git a/Contributing-code-to-Oppia.md b/Contributing-code-to-Oppia.md index 7f011cd1..44e14eb9 100644 --- a/Contributing-code-to-Oppia.md +++ b/Contributing-code-to-Oppia.md @@ -56,7 +56,7 @@ Additionally, GitHub also provides an advanced search syntax that allows you to 7. Take up your first Oppia starter issue! (See [below](https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#finding-something-to-do) on how to do this.) Make sure to read and follow the [[PR instructions|Rules-for-making-PRs]] closely so that your PR review proceeds smoothly. - * In your browser, consider bookmarking the [[guide to making pull requests|Rules-for-making-PRs]] for easy reference later, as well as the ["my issues" page](https://github.com/issues/assigned) (so that you can keep track of the issues assigned to you). + * In your browser, consider bookmarking the [[guide to making pull requests|Rules-for-making-PRs]] for easy reference, as well as the ["my issues" page](https://github.com/issues/assigned) (so that you can keep track of the issues assigned to you). * Facing any problems (including non-coding ones)? Please feel free to create a [GitHub Discussion](https://github.com/oppia/oppia/discussions) and get help from the Oppia community. You can use this avenue for asking anything -- questions about any issue, who to contact for specific things, etc. @@ -66,6 +66,8 @@ Additionally, GitHub also provides an advanced search syntax that allows you to See our [[page of learning resources|Learning-Resources]] for suggestions on how you can improve your development skills. When you take up an issue that requires programming languages or tools you are unfamiliar with, check out that page for resources that other developers have found useful when learning. +Our wiki has lots of useful documentation, including **tutorials** which are denoted in the sidebar by 👣 icons. Use them to learn key skills that are needed for solving issues. + We also **strongly recommend** looking through the resources under "Developing Oppia" in the wiki sidebar. Good places to start include the [[Overview of the Oppia codebase|Overview-of-the-Oppia-codebase]] and the [[tips on how to find the right code to change|Find-the-right-code-to-change]]. ## Finding something to do @@ -76,14 +78,18 @@ Welcome! Please make sure to follow the instructions above if you haven't alread After that, you can choose a good first issue from the [list of good first issues](https://github.com/oppia/oppia/labels/good%20first%20issue). These issues are hand-picked to ensure that you don't run into unexpected roadblocks while working on them, and each of them should have clear instructions for new contributors. If you see one that doesn't, please let us know via [GitHub Discussions](https://github.com/oppia/oppia/discussions) and we'll fix it. For other issues, you might need to be more independent because we might not know how to solve them either. -Please only work on issues that are labelled **"Impact: High"** or **"Impact: Medium"**. (We are not focusing on "Impact: Low" or "Backlog" issues for now; if you submit a fix for those, you will likely be redirected. We also recommend not working on "triage needed" issues since these might get closed or modified during the weekly triage process.) +Please only work on issues that are labelled **"Impact: High"** or **"Impact: Medium"**. + +> [!CAUTION] +> - Do not work on "Impact: Low" or "Backlog" issues. If you submit a fix for those, you will probably be asked to pick a different issue. Those issues often haven't been looked at closely yet, and might not be priorities for implementation. +> - Do not work on issues with the "triage needed" label, including issues that you recently filed. These issues haven't been vetted yet and might get closed or modified during the weekly triage process. As a new contributor, if you run into any problems along the way, we're here to help! Check out our [[Getting Help Page|Get-help]] for the communication channels you can use. You can also browse good first issues for each of the core Oppia Web teams to find something you'd enjoy working on! Please only choose issues that have **not yet** been assigned, unless the issue is a "checkbox issue" with multiple claimable parts. Here are the project boards for the different teams: -- Developer Workflow: https://github.com/orgs/oppia/projects/8/views/10, typically backend or frontend - Learner and Creator Experience (LaCE): https://github.com/orgs/oppia/projects/3/views/10, typically frontend or full-stack +- Developer Workflow: https://github.com/orgs/oppia/projects/8/views/10, typically backend or frontend - Contributor Dashboard: https://github.com/orgs/oppia/projects/18/views/14, typically frontend or full-stack ### How to tackle good first issues @@ -91,22 +97,28 @@ You can also browse good first issues for each of the core Oppia Web teams to fi When you've found a good first issue you'd like to tackle, please investigate it first to understand why the issue is happening. Here are some things you should do: - Read the entire discussion thread to understand what has been tried so far. -- Try to reproduce the issue on your local dev server. (For Contributor Dashboard issues, the [[Contributor Dashboard onboarding guide|Contributor-dashboard]] has some useful setup information. For Learner and Creator Experience releated issues, you can refer to the [[LaCE onboarding guide|LaCE-onboarding-guide]].) -- Figure out why the problem is happening, and find the relevant code in the Oppia repository to change. (The [['Finding the right code to change' wiki page|Find-the-right-code-to-change]] might be helpful.) If you have trouble with this, feel free to ask on [GitHub Discussion](https://github.com/oppia/oppia/discussions) and explain what you've tried doing so far. -- If the issue is easy to fix, try to get a rough fix working on your local dev server! +- Try to reproduce the issue on your local dev server. (For Contributor Dashboard issues, the [[Contributor Dashboard onboarding guide|Contributor-dashboard]] has some useful setup information. For Learner and Creator Experience related issues, you can refer to the [[LaCE onboarding guide|LaCE-onboarding-guide]].) +- Figure out why the problem is happening, and [find the relevant code in the Oppia repository to change|Find-the-right-code-to-change]. If you have trouble with this, feel free to ask on [GitHub Discussion](https://github.com/oppia/oppia/discussions) and explain what you've tried doing so far. +- Try and get a fix working on your local dev server. You will need to do this in order to claim the issue (see below). Once you have a good understanding of the issue, you can ask for it to be assigned to you by leaving a comment as follows: -- Explain clearly how you'd tackle the issue (at a minimum, point to which file(s) you'd modify and describe the changes you'd make). If possible, show a screenshot or code snippet demonstrating your proposed fix. +- Show a video of the fix working correctly on your local machine. (For user-facing changes, your video should show a URL that starts with localhost:8181.) +- Explain clearly what you did to tackle the issue (at a minimum, point to which file(s) you modified and describe the changes you made). You can include code snippets if you like. - @-mention the leads of the corresponding project (you can find their details [here](https://github.com/orgs/oppia/projects)), letting them know you'd like to work on it and when you can submit a PR by. -If your explanation makes sense, we'll assign the issue to you. Once assigned, feel free to submit a PR by following the [[instructions for making a PR|Rules-for-making-PRs]]. We recommend actively working to get your PR merged once you are assigned, because we will de-assign contributors if the PR is closed for being stale or there is no activity after the initial assignment. +If your proof looks good and your explanation makes sense, we'll assign the issue to you. Once assigned, feel free to submit a PR by following the [[instructions for making a PR|Rules-for-making-PRs]]. We recommend actively working to get your PR merged once you are assigned, because we will de-assign contributors if the PR is closed for being stale or there is no activity after the initial assignment. + +> [!IMPORTANT] +> Please follow the [[PR instructions|Rules-for-making-PRs]] carefully, otherwise your PR review may be delayed or your PR may be closed. + +#### Getting help -Generally, we aim to review PRs within 48 hours. If you have not heard from a reviewer by then, feel free to escalate. You can leave a comment on the review thread and also add a message to the "Contacting Folks" section of [GitHub Discussions](https://github.com/oppia/oppia/discussions/categories/q-a-contacting-folks). +Generally, we aim to review PRs within 48 hours. If you have not heard from a reviewer within 48 hours, feel free to leave a comment on the review thread, and also add a message to the "Contacting Folks" section of [GitHub Discussions](https://github.com/oppia/oppia/discussions/categories/q-a-contacting-folks). **Don't wait longer than 2 days to do this.** -If you run into any problems, feel free to create a [GitHub Discussion](https://github.com/oppia/oppia/discussions) and get help from the Oppia community, or [request a mentor](https://forms.gle/udsRP4WQgLcez9Zm8) if you'd like individual support. If you need specific help, write a [[debugging doc|Debugging-Docs]] to compile the necessary information and outline your thought process, so that we can help you more easily. +If you run into a general problem, please create a [GitHub Discussion thread](https://github.com/oppia/oppia/discussions) to get help from the Oppia community. -**Important Note**: Please follow the [[PR instructions|Rules-for-making-PRs]] carefully! Otherwise your PR review may be delayed or your PR may be closed. +**Note:** It is important to pick a starter issue that you are able to do! If you find that the issue you are tackling is too difficult, it is fine to choose another one instead and come back to it later. ### Contributor Roles diff --git a/LaCE-onboarding-guide.md b/LaCE-onboarding-guide.md index 28703e3d..634b02cb 100644 --- a/LaCE-onboarding-guide.md +++ b/LaCE-onboarding-guide.md @@ -1,7 +1,11 @@ This page will help you solve common queries that you may face while working on LaCE issues. If you do not get a satisfactory explanation from this page, feel free to mention your query in the issue thread that you are working on or else make a discussion regarding this in [Dicussions](https://github.com/oppia/oppia/discussions). If you feel that your query can be a valid addition to this page, then make sure to create a new issue [here](https://github.com/oppia/oppia-web-developer-docs/issues) addressing your question. +For help with tackling specific issues, you can also write a [[debugging doc|Debugging-Docs]] to organize your thoughts, and share it on [GitHub Discussions](https://github.com/oppia/oppia/discussions/categories/q-a-debugging-docs). Doing this will make it easier for others to suggest next steps. + # How to perform common operations on your local server +Please note that, to create a skill or a topic, you will need the Curriculum Admin role. To get this role, navigate to the Admin roles tab (/admin) and assign yourself the correct role from the Roles tab. + | Index | Common Queries | | :---: | :---: | | 1. | [Creating a Topic](#1-creating-a-topic) | diff --git a/Launching-new-features.md b/Launching-new-features.md index fb45ad22..1146ebd0 100644 --- a/Launching-new-features.md +++ b/Launching-new-features.md @@ -1,43 +1,51 @@ -When developing a new feature, you might want to limit the scope of the feature so that it's only enabled when certain criteria are met (e.g. only enabled in dev environment). In these cases, you can use the feature gating system to gate the enabling of the features with a feature flag. +This wiki page explains how we build and launch new features safely at Oppia. In particular, user-facing features which need more than one PR to implement, or that aren't yet fully tested, **must** be launched using the process described on this page. -## Why do we need feature flags? +To control how a feature is launched, we use **feature flags** to limit the scope of the feature so that it's only enabled when certain criteria are met (e.g. only enabled in the dev environment, or on the test server). This means that developers can hide features that are still in development or still being tested. -Feature flags vastly improve/simplify the development process by allowing developers to safely deploy new features to production. They also allow us to quickly disable features if we find that they are causing problems in production. +Using a feature flag has several advantages: -They also allow us to hide features that are still in development/still being tested. This means that developers can add the feature incrementally to the codebase, and only enable it once it is fully ready for production. Should the feature break, they also allow us to easily disable it without having to remove all the changes from the codebase. This is especially useful when the feature is large and/or complex. +- It allows the developer to add the feature incrementally to the codebase, and only enable it once it is fully ready for production. -Additionally, feature flags allow us to decouple the feature & binary releases. This allows us to deploy new binaries to the server with less risk of regressions. +- Should the feature break or cause problems in production, we can easily disable the feature without having to remove all the changes from the codebase. This is especially useful when the feature is large and/or complex. +- They allow us to decouple the feature and binary releases. This allows us to make new code deployments to the server with less risk of regressions. -## How do you, as a developer, use feature flags? +## How to use feature flags -### When to use feature flags? +If you are working on a large scale user-facing feature that will take more than 1 PR to fully implement, please follow the steps listed below to gate your feature appropriately: -Feature flags should be used when you are working on a feature whose scale makes it hard to implement all at once, or for more experimental features that are likely to cause breakages. Essentially, feature flags are a must for features that are not yet ready for production/fully-tested at the time that they're merged into develop. +1. Create a [feature testing doc](https://docs.google.com/document/d/1ibQC9t1jnOg-o1lrHEM3QEXeF4eSqAiPhs2lD2iCpy4/edit?tab=t.0) that outlines all the critical user journeys (CUJs) for the fully-completed feature. Share this doc with the QA leads and Product Operations teams. -### How to use feature flags? +2. In your very first PR for the feature: -Say you are working on a large scale user-facing feature that will take 1 or more PRs to fully implement. In such a case, please use feature flags to gate your feature appropriately by carefully following the steps listed below: + - Introduce a new feature flag to the codebase, that is meant to be used with this new feature. This 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 test/prod servers). -1. The very first PR you make must introduce a new feature flag to the codebase, 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 test/prod servers). 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. + - Add a link in your PR description to the feature testing doc that you created in Step 1. -2. The first PR above must be merged before any of the following PRs are merged. This is to ensure that the feature flag is available in the codebase for it to be used in the following PRs. +3. While developing your feature: -3. When developing, make sure that e2e tests are present for your feature. If the feature is gated behind a feature flag, you may need to use the [enableFeature](https://github.com/oppia/oppia/blob/c48ff1510a680666bfe891d7b2f68130d21e4dcf/core/tests/webdriverio_utils/ReleaseCoordinatorPage.js#L126) utility functions for the release-coordinator page to first enable the required flag, and then proceed to perform the testing. For unit tests, you should include tests for both the `flag=True` and `flag=False` cases. + - Ensure that every single user-facing aspect of the feature -- whether frontend or backend -- is gated behind the feature flag (both in the first PR, and all the following ones as well). It is important to ensure that all the new functionality is fully gated and does not "leak", otherwise this could cause issues like data corruption. -4. The very last PR you make (to finish up the feature you are working on) must include changes that move the feature flag to the TEST stage. This is to ensure that the feature is available in the test environment, so that we can feature-test it before it is made available to the users in the production environment. **NOTE: Please test all the changes manually to make sure that the feature works fully end-to-end on your local dev server, before moving the flag to the TEST stage.** + - Make sure to write e2e/acceptance tests are present for your feature. You may need to use the [enableFeature](https://github.com/oppia/oppia/blob/c48ff1510a680666bfe891d7b2f68130d21e4dcf/core/tests/webdriverio_utils/ReleaseCoordinatorPage.js#L126) utility functions for the release-coordinator page to first enable the required flag, and then proceed to perform the testing. For unit tests, you should include tests for both the `flag=True` and `flag=False` cases. -5. Once your PR has reached a stage where you don't anticipate any more major code changes, fill out [this form](https://forms.gle/zDCsoN6Xb6JvQku87) to request that your feature be tested. This form will be processed by the server admins team, who will work with you and the feature testers to set a date during which your feature will be available to use on one of our test servers. (We can only surface the feature for a limited time because the servers are also needed for other things, such as release testing.) Once the testing date is confirmed, the server admin will make a deployment, turn the feature flag on, and send instructions to the feature testers for how to test the feature. Feature testers will use this [feature review template](https://docs.google.com/document/d/1ibQC9t1jnOg-o1lrHEM3QEXeF4eSqAiPhs2lD2iCpy4/edit) to conduct the testing. +4. Once your feature is code-complete, create a PR that moves the feature flag to the TEST stage. This allows the feature to be tested by our QA team before it is made available to the users in the production environment. In your PR, link to the feature testing doc that you created in Step 1. **NOTE: Please test all the changes manually to make sure that the feature works fully end-to-end on your local dev server, before moving the flag to the TEST stage.** -6. If the feature testing reveals that the feature is not yet ready for production, you must work on fixing the highlighted issues before proceeding further. You can request a re-test once all the testing feedback is addressed. +5. Once the above PR is merged, fill out [this form](https://forms.gle/zDCsoN6Xb6JvQku87) to request that your feature be tested. The Product Operations team will then organize testing for your feature, using the feature review template that you created in step 1. If the testing reveals that the feature is not yet ready for release, you must fix the highlighted issues before proceeding. You can request a re-test once you have addressed all the testing feedback. -7. Once you receive a go-ahead from the feature testers, you must merge another PR. This PR should do only one thing, i.e. move the feature flag to the PROD stage, allowing it to be enabled/disabled in production (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 with the release coordinators that the new CUJs for this feature have been added to the overall CUJs used for testing releases in general. +6. Once your feature passes testing, do all of the following: -8. Once this PR is merged, send a ["job run 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. - - (Optional) If you like, you can fill in [this form](https://goo.gl/forms/sNBWrW03fS6dBWEp1) to announce your feature to the public once it's launched! + - Create a PR that moves the feature flag to the PROD stage (and does nothing else). This will allow your feature to be launched in production. **NOTE: When opening this PR, include a link to the testing doc or other proof that the feature has been approved for release.** + + - Send a ["feature-flag flip request"](https://forms.gle/rUJaHJSpRGemtGDp6) to the release coordinators to turn the flag on in production. + + - Ensure that the QA team has added the CUJs for the new feature to the overall CUJs used for testing regular releases. + + - (Optional) If you like, you can fill in [this form](https://goo.gl/forms/sNBWrW03fS6dBWEp1) to announce your feature to the public once it's launched! + +7. Once the feature is confirmed to be functioning as intended in production (for at least 2 weeks) by the product team, please do the following, in order: -9. Once the feature is confirmed to be functioning as intended in production (for at least 2 weeks) by the product team, please do the following, in order: - Make sure that the feature is ready to be made permanent. To do this, confirm with the PMs that no users have reported issues with it, and that no regressions have been detected via StackDriver or general user feedback. The PMs should also fill in this [post-launch review template](https://docs.google.com/document/d/1DifFAe3oRzjmVPh2fEllfAky4n0QMAXVQc3Y580qkr8/edit). + - Once you have confirmation that the feature can be made permanent, merge one last PR to "un-gate" the feature and move the feature flag to the deprecated stage (one of the stages listed in `core/feature_flag_list.py`, meant for flags that are no longer in use). Additionally, in the same PR, please remove all remaining references to the feature flag from the codebase (for example, in all the `if` blocks you created to gate the feature).