Skip to content

Oppia's code owners and checks to be carried out by developers

Jump to bottom
Hardik Goyal edited this page May 16, 2024 · 8 revisions

At Oppia, we constantly try to improve the code quality and the experience for both the developers and the end users. This requires many optimizations and checks to make sure that things don't break down or cause any degradation to the users' experience. This is why we have a group of code-owners for specific folders and files who keep a check on the changes to make sure all goes smoothly.

Also, the developers who are making the changes to any files must bear in mind the checks they need to perform on their changes making sure it conforms to the rules the folder/file is bound to.

The following is a list of the code-owners and the checks the code-owner looks for in the corresponding folders/files.

@prasanna08

Answer classification team.

Files Checks carried out
/core/controllers/classifier*.py Ensure that Signature verification algorithm is in sync with signature generation algorithm at Oppia-ml. This code should not break and end to end tests (synchronization between Oppia and Oppia-ml) must be carried out manually by the developer if the code is to be changed.
/core/domain/classifier*.py Most of the functionality has automated tests but ensure that logic for fetching new jobs (used by Oppia-ml to poll for jobs) and logic for when to submit a new job should stay intact.
/core/storage/classifier/ This module is mostly covered by automated tests.
/core/templates/domain/classifier/ These are domain files used by prediction services to store the prediction result. They are supported by automated tests.
/extensions/classifiers/ These files (are most crucial and) use the trained model stored by Oppia-ml on Oppia.
  • Hence, data format used by these files should be in sync with those submitted by Oppia-ml.
  • If the code is changed then end to end tests must be carried out to ensure that Oppia and Oppia-ml are in synch.
  • In addition to end to end tests, developer must also verify that the prediction output by these services are same as one that would be generated by Oppia-ml, if it were to do prediction. This is important because Oppia-ml uses libraries for training whereas these files are hand coded and their accuracy should match to the one generated by libraries.

@DubeySandeep

Audio and Translation team.

Files Checks carried out
/core/controllers/translator*.py This comes under the audio translation project and needs an approval from the audio translation team:
  • General structure of translation workflow and it's coverage in the exploration.

  • UI changes in the translation tab.
    • /core/templates/pages/exploration_editor/translation_tab/

    Exploration project.

    /core/controllers/editor*.py Ensure the structure of exploration is kept intact:
    • Backend/UI:
      • The domain and controller levels are kept in a similar structure.
      • The new changes add relevant unit tests.
      • Migration structure is kept intact.
      • The UI of the different tabs are kept intact and new changes adds relevent e2e tests.
    /core/domain/exp*.py
    /core/templates/domain/editor/
    /core/templates/domain/exploration/
    /core/templates/pages/exploration_editor/editor_tab/
    /core/templates/pages/exploration_editor/history_tab/
    /core/templates/pages/exploration_editor/settings_tab/

    Forms pages.

    /core/templates/components/forms/ This contains schema-based editor and forms related to audio projects, we are not expecting to have a lot of changes around this files.

    Miscelleneous.

    /core/templates/domain/utilities/ This mostly has Object factory related to the audio project. I'll be reviewing this as part of the audio project.

    @kevinlee12

    Collection project.

    Files Checks carried out
    /core/controllers/collection*.py Check that code is accompanied with the relevant tests; code aligns with coding patterns.
    /core/domain/collection*.py Check that code is accompanied with the relevant tests; code aligns with coding patterns.
    /core/storage/collection/ No existing changes are expected.
    /core/templates/domain/collection/ Check that code is accompanied with the relevant tests; code aligns with coding patterns.
    /core/templates/pages/collection_editor/ Ensure that collection editors have a positive UX experience. Check that code aligns with existing code patterns.
    /core/templates/pages/collection_player/ Ensure that collection editors have a positive UX experience. Check that code aligns with existing code patterns.

    Interaction Project.

    /core/templates/domain/objects/ Ensure that objects are related to existing interactions. In general, no changes are expected for existing interactions.
    /extensions/interactions/ Ensure that users have a positive UX experience. For new interactions, ensure that (informal) UX studies are done. For existing interactions, changes are expected to be minimal except for styling and bug fixes.

    Library Page.

    /core/controllers/library*.py No changes are expected here.
    /core/templates/pages/library/ Check that UX for the users is positive. Changes should be compatible with mobile.

    User's profile page.

    /core/templates/domain/user/ Ensure that users have a positive UI/UX experience (ie. metrics are easily seen, etc).
    /core/templates/pages/profile/ Ensure that users have a positive UI/UX experience (ie. metrics are easily seen, etc).

    @nithusha21

    Dashboard frontend pages.

    Files Checks carried out
    /core/controllers/creator_dashboard*.py Review changes from the creator UX perspective. Make sure code is functionally correct, readable, and formatted correctly. Code must be tested using unit tests.
    /core/templates/domain/creator_dashboard/
    /core/templates/pages/creator_dashboard/
    /core/templates/domain/learner_dashboard/ Review changes from the learner UX perspective. Make sure code is functionally correct, readable, and formatted correctly. Code must be tested using unit tests.
    /core/templates/pages/learner_dashboard/
    /core/templates/pages/email_dashboard/ Changes to UI should be tested. Make sure the UX for users is good.
    /core/templates/pages/notifications_dashboard/ Changes to UI should be tested. Make sure the UX for users is good.

    Suggestion project

    /core/controllers/suggestion*.py This comes under the review system project, and hence needs to be reviewed by Nithesh. Things to look out for:
    • Generalizability across various use cases.
    • Alignment with existing patterns in generalized review system code
    • Reduced code duplication, and simplified general code.
    • No new model additions are expected, the existing GeneralSuggestionModel is expected to be reused.
    /core/domain/suggestion*.py
    /core/storage/suggestion/
    /core/templates/domain/suggestion/
    /core/templates/pages/suggestion_editor/

    Feedback project.

    /core/controllers/feedback*.py This comes under the review system project and needs to be reviewed by Nithesh. Things to look out for:
    • Generalizability across various use cases.
    • Reduced code duplication, and simplified general code.
    • No changes to existing models is expected.
    • Changes to UI must include changes for the learner interface and creator interface (to give and view feedback).
    /core/domain/feedback*.py
    /core/storage/feedback/
    /core/templates/domain/feedback_message/
    /core/templates/domain/feedback_thread/
    /core/templates/pages/exploration_editor/feedback_tab/

    Navigation bar project.

    /core/templates/domain/sidebar/ No significant changes are expected here. As this serves as the general navigation strategy on Oppia, this should be well tested, and user-friendly.

    The buttons on the various navigation bars should ideally not change, changes to these files would either touch functionality of existing buttons, or styling changes.
    /core/templates/components/create_button/
    /core/templates/components/side_navigation_bar/
    /core/templates/components/top_navigation_bar/

    QA Team.

    @U8NWXD

    /core/tests/ For e2e tests:
    • Make sure they run optimally. Any common setup needs to be written in BeforeAll blocks.
    • The test follows a user's path for using the feature.
    • Enough assertions (and meaningful ones) are made, which ensures that the state the test has reached matches the expected state.
    • There should be no manipulation using the code. The complete code should test a feature using just the UI.
    • The tests should be granular, and test a specific functionality. E.g, add a hint, add a translation, etc.
    • Tests must cover non-happy paths (error-prone paths) if applicable. Like, what happens when the uploaded audio is too big?

    For webdriverio_utils:

    • Make sure style guidelines and naming guidelines are followed.
    • All webdriverio classes must start with e2e-test-.

    For webdriverio_mobile, the tests should run successfully on Browserstack.

    Also make sure your PR meets all the points in the codeowners checklist on the wiki.

    @apb7

    Dev Workflow team

    Files Checks carried out
    /scripts/ For run_lint_checks.py, pylint_extensions.py and pylint_extensions_test.py, the added lint checks should be optimized and placed at the best location possible within the script.

    For other scripts, in general, the changes done should be optimum and do not affect the existing infrastructure negatively.

    @aks681

    Exploration project.

    Files Checks carried out
    /core/storage/exploration/ Any changes here should have corresponding changes in exp_domain and everywhere else where explorations are used.
    /core/templates/domain/state/
    • Changes here should not break explorations or questions. Can be verified in their respective editors.
    • Corresponding tests should be added.
    /core/templates/domain/state_card/
    /core/templates/pages/exploration_editor/preview_tab/
    • Make sure the code changes here are functionally correct with appropriate tests and that the UI changes here are intuitive.
    • Any additional fields that are being added to the models should have the changes reflected in the respective domain and service files.
    /core/templates/pages/exploration_player/
    /core/templates/pages/state_editor/
    /core/templates/components/state/

    Question project.

    /core/controllers/question*.py Any backend changes here should be functionally correct, have the corresponding tests and shouldn't break any existing functionality.
    /core/domain/question*.py
    /core/storage/question/
    /core/templates/domain/question/ Any changes to the question editor should be functionally correct and there should be extra care taken to not break the topic or skill editor as both of them use these directives.
    /core/templates/pages/question_editor/
    /core/templates/pages/questions_list/

    Skill project.

    /core/controllers/skill*.py Any backend changes here should be functionally correct, have the corresponding tests and shouldn't break any existing functionality.

    The corresponding class in skill_domain and the functions related to it in skill_services should be updated for any new field added to it.

    /core/domain/skill*.py
    /core/storage/skill/
    /core/templates/domain/skill/ Same as above, any new field added to skill should be reflected in one of the ObjectFactories.

    UI changes should be intuitive and shouldn't break existing functionality.

    /core/templates/pages/skill_editor/

    Story project.

    /core/controllers/story*.py Any backend changes here should be functionally correct, have the corresponding tests and shouldn't break any existing functionality.

    The corresponding class in story_domain and the functions related to it in story_services should be updated for any new field added to it.

    /core/domain/story*.py
    /core/storage/story/
    /core/templates/domain/story/ Same as above, any new field added to story should be reflected in one of the ObjectFactories.

    UI changes should be intuitive and shouldn't break existing functionality.

    /core/templates/pages/story_editor/

    Topic project.

    /core/controllers/topic_*.py Any backend changes here should be functionally correct, have the corresponding tests and shouldn't break any existing functionality.

    The corresponding class in topic_domain and the functions related to it in topic_services should be updated for any new field added to it.

    /core/domain/topic*.py
    /core/storage/topic/
    /core/templates/domain/topic/ Same as above, any new field added to topic should be reflected in one of the ObjectFactories.

    UI changes should be intuitive and shouldn't break existing functionality.

    /core/templates/domain/topic_viewer
    /core/templates/pages/topic_editor/
    /core/templates/pages/topic_viewer/

    Topics and skills dashboard project.

    /core/controllers/topics_and_skills_dashboard.py Changes here should have their corresponding tests.
    /core/templates/domain/topics_and_skills_dashboard/ There aren't many major changes expected here, but any that are there shouldn't break existing functionality and should be intuitive to the user.
    /core/templates/pages/topics_and_skills_dashboard/

    @ankita240796

    Global components.

    Files Checks carried out
    /core/templates/components/alerts/
    • All variables are declared before use
    • No multiple declarations of same variable are present
    • Subsequent variable usage has the same type
    • No new properties are added to a declared variable
    • Loop variables are declared
    • If scope is used in link function, proper typings are added to custom-scope-defs.d.ts
    • Each file contains a single component (we have a lint check for this too)
    • Existing functionality does not break
    /core/templates/components/attribution_guide/
    /core/templates/components/background/
    /core/templates/components/embed_modal/
    /core/templates/components/loading/
    /core/templates/components/profile_link/
    /core/templates/components/promo/
    /core/templates/components/share/
    /core/templates/components/social_buttons/
    /core/templates/components/summary_tile/

    Global stylesheet.

    /core/templates/css/oppia.css Not much to check apart from that the changes don't create any issues in the existing UI. Other css linter stuff is followed. \ Proper comments are present if certain values are hardcoded.

    @seanlip

    Interaction project.

    Files Checks carried out
    /core/templates/expressions/ Familiar with expressions based on earlier work on parameters. This part of the code is largely unused currently so I expect to see no changes.
    /extensions/dependencies/ Want to make sure I understand what new dependencies we are taking on so that I can check that we are properly licensing them etc.
    /extensions/objects/ Want to ensure that we don't get an overpopulation of objects.
    /extensions/value_generators/ This should be unused; want to make sure that it remains unused.

    Lesson Analytics Team.

    +@brianrodri

    /core/domain/stats*.py As owner of the Statistics tab, Brian should review all code that goes in here.
    /core/storage/statistics/ As owner of the Statistics tab, Brian should review all code that goes in here.
    /core/templates/domain/statistics/ As owner of the Statistics tab, Brian should review all code that goes in here.
    /core/templates/pages/exploration_editor/improvements_tab/ As owner of all aspects of the Improvements tab, Brian should review all code that goes in here.
    /core/templates/pages/exploration_editor/statistics_tab/ As owner of the Statistics tab, Brian should review all code that goes in here.
    /extensions/actions/ This is a part of Playthroughs functionality, which Brian owns.
    /extensions/answer_summarizers/ This is a part of Statistics display functionality, which Brian owns.
    /extensions/issues/ This is a part of Playthroughs functionality, which Brian owns.
    /extensions/visualizations/ This is a part of Statistics display functionality, which Brian owns.

    Infrastructure

    /core/storage/ Ensure that storage models are not storing data that we do not wish to collect in the first place.
    /core/platform/ In general, changes are not expected at this layer, so I want to review this to ensure that all changes made here are actually necessary.

    Miscellaneous

    /core/domain/html*.py These files typically cover HTML sanitization which needs to be handled carefully from a security perspective.
    /core/templates/pages/about/ This page is how Oppia presents itself to the world, so I would like to keep abreast of any changes that are made to the text.
    /core/templates/pages/privacy/ Any changes to this are sensitive and should be approved by a data privacy lead (Ben or Sean) before being merged.
    /core/templates/pages/terms/ Any changes to this are sensitive and should be approved by a data privacy lead (Ben or Sean) before being merged.
    /core/templates/pages/tests/ Miscellaneous test files; in general, no changes are expected here.
    /export/ In general, no changes are expected here.

    Restricted pages.

    /core/controllers/admin*.py In general, no changes are expected here.
    /core/templates/pages/admin/ In general, no changes are expected here.
    /core/templates/pages/moderator/ In general, no changes are expected here.
    /.github/CODEOWNERS This file defines the code-owner responsibilities for the entire Oppia codebase, and I would like to make sure that each file/folder is assigned to someone who has enough knowledge/understanding of the code in it so that they can review it well.

    @bansalnitish

    Rich text editor team.

    Files Checks carried out
    /core/templates/components/CkEditorRteDirective.js It's very important to check that the editor works perfectly. A lot of attention should be paid to the UI.
    /extensions/rich_text_components/ It's very important to check that the editor works perfectly. A lot of attention should be paid to the UI.
    /extensions/ckeditor_plugins/ It's very important to check that the editor works perfectly. A lot of attention should be paid to the UI.

    No unnecessary plugin should be added.

    @vojtechjelinek + @Jamesjay4199

    Simple pages.

    Files Checks carried out
    /core/templates/pages/contact-page/ Check that UX for the users is positive. Changes should be compatible with mobile.
    /core/templates/pages/donate-page/
    /core/templates/pages/error-pages/
    /core/templates/pages/get-started-page/
    /core/templates/pages/landing-page/
    /core/templates/pages/maintenance-page/
    /core/templates/pages/preferences-page/
    /core/templates/pages/signup-page/
    /core/templates/pages/splash-page/
    /core/templates/pages/teach-page/
    /core/templates/pages/thanks-page/

    Speed Improvement team.

    Generally: Ensure that the modifications to these files don't affect Oppia performance and speed.

    /app.yaml Ensure that:
    • cache timeouts are not changed
    • new handlers have correct cache timeouts values
    /dependencies.json (+@seanlip) (seanlip) Ensure that no new libraries are added unnecessarily, and that third-party libraries have a proper license.
    /core/templates/pages/footer_js_libs.html This file shouldn't be modified at all for now.
    /core/templates/pages/header_css_libs.html This file shouldn't be modified at all for now.
    /core/templates/pages/header_js_libs.html This file shouldn't be modified at all for now.
    webpack.common.config.ts Changes that are common for dev and prod should be done in webpack.common.config.ts. All the changes should account for the speed of the webpack build.
    webpack.dev.config.ts
    webpack.prod.config.ts

    Temporary codeowner transfer process

    If you will be unavailable for more than 24 hours, please transfer your ownership with a delegate, follow the process detailed below:

    1. Create an issue using the "Temporary codeowner transfer" template on Github. [Note down the ISSUE_NUMBER.]
    2. Do the changes in CODOWNER file and add a top level comment: TODO(#ISSUE_NUMBER): Revert ownership to @USERNAME after YYYY-MM-DD.
    3. Create a PR with the title: Fix part of #<ISSUE_NUMBER>: Temporary codeowner transfer
    4. Assign the contributor whom you have transferred the responsibility as reviewer for the PR. [Make sure to get this PR merged before you leave.]
    5. [Once you are back] Undo the old changes in CODEOWNER file and remove the TODO comment.
    6. Create a revert PR of the old-codeowner transfer PR with title Fix #<ISSUE_NUMBER>: Revert temporary codeowner transfer.
    7. Assign the contributor whom you had transferred the responsibility as a reviewer for the PR.
    8. Get the revert PR merged!

    Have an idea for how to improve the wiki? Please help make our documentation better by following our instructions for contributing to the wiki.

    Core documentation

    Developing Oppia

    Developer Reference

    Clone this wiki locally