Skip to content

History / Static Analysis Checks

Revisions

  • Fix #5312: Make todo open check locally runnable [Blocked: #5313] (#5315) ## Explanation Fixes #5312 This updates the TODO open check script to be locally runnable rather than requiring the developer to manually download the list of issues from GitHub to analyze. This simplifies things and allows the script to be easily run within the ``static_checks.sh`` script. This is being done via interacting directly with GitHub using its RESTful API (see https://docs.github.com/en/rest/issues/issues?apiVersion=2022-11-28#list-repository-issues) in conjunction with the user's local auth token used to set up their ``gh`` tool (which needs to be set up, hence the changes to the wiki documentation and clear error messages from the new ``GitHubClient`` utility). To further simplify things, a ``regenerate`` mode was added to regenerate the TODO exemptions textproto file (which is helpful for #4929 hence why this comes before that PR). The new command syntax to perform the TODO check is: ```sh bazel run //scripts:todo_open_check -- <path_to_dir_root> <path_to_proto_binary> [regenerate] ``` With a specific example to just perform the check: ```sh bazel run //scripts:todo_open_check -- $(pwd) scripts/assets/todo_open_exemptions.pb ``` And an example to also perform regeneration: ```sh bazel run //scripts:todo_open_check -- $(pwd) scripts/assets/todo_open_exemptions.pb regenerate ``` Some other things specifically to note: - TODO exemptions needed to be updated in this PR due to TODO utility & test changes. The new file was created using the new regenerate functionality. - The TODO check has been added to the end of ``static_checks.sh``. - The GitHub CI workflow was updated to use the new script syntax appropriately. - This is the first time scripts have been updated to integrate with Retrofit, and this setup is going to be reused in the future for other services. - The data model for issues has been updated to better represent the remote data structure. - Moshi is being used along with Retrofit for an easier interaction with GitHub as a remote endpoint. All of this has been wrapped in a new ``GitHubClient``. - ``GitHubClient`` is designed to download all issues regardless of length (whereas before the manual download step was limited to the first 2000 issues of the repository) using pagination. - New tests were added to verify the regenerate flow (and properly set up the mock OkHttp server since the script now relies on an HTTP endpoint to download the GitHub issues itself). - ``GitHubIssue`` is exempted from tests since it's just a basic data structure, so there's no specific logic to test. - ``GitHubService`` is exempted from tests since it's a template to generate code via Retrofit's annotation processor, so there's no specific logic to test. - All scripts proto libraries were updated to use normal Java (rather than Java lite) generation to provide text format support. - The file paths included in ``TodoOpenCheck``'s output has been simplified to be relative to the repository root rather than using absolute paths (for parity with many other app scripts). - Since ``GitHubClient``'s tests required interacting with ``gh``, a new ``FakeCommandExecutor`` was added (along with its own tests) to provide support for orchestrating local utilities. This may be useful in other tests in the future, though some of those script tests intentionally integrate with environment commands like ``git`` and ``bazel``. ## Essential Checklist - [x] 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: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only This is an infrastructure-only PR. --------- Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com> Co-authored-by: Sean Lip <sean@seanlip.org>

    @BenHenning BenHenning committed Mar 20, 2024
  • Fix #5192, #4610 : [Developer Video] Understanding CI check failures & Replace wiki broken links (#5199) <!-- READ ME FIRST: Please fill in the explanation section below and check off every point from the Essential Checklist! --> ## Explanation Fix #5192, #4610 : [Developer Video] Understanding CI check failures & Replace Wiki broken links This PR include developer video that explains how to handle failures in our Continuous Integration (CI) checks in oppia-android. It also includes the replacement of broken links in our Wiki, improving the documentation for a smoother developer experience. <!-- - Explain what your PR does. If this PR fixes an existing bug, please include - "Fixes #bugnum:" in the explanation so that GitHub can auto-close the issue - when this PR is merged. --> ## Essential Checklist <!-- Please tick the relevant boxes by putting an "x" in them. --> - [x] 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](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

    @MohitGupta121 MohitGupta121 committed Oct 20, 2023
  • Fix part of #4391: Fix failing build on m1 macs (#5111) <!-- READ ME FIRST: Please fill in the explanation section below and check off every point from the Essential Checklist! --> Fix part of #4391 : Fix failing build on M1 Macs ## Explanation <!-- - Explain what your PR does. If this PR fixes an existing bug, please include - "Fixes #bugnum:" in the explanation so that GitHub can auto-close the issue - when this PR is merged. --> Some M1 Macs fail to build the Oppia app because of unused imports in some of the proto files in the project. This PR attempts to fix this issue by removing unused imports in proto files. Files affected by this fix are; - profile.proto - deprecation.proto This is a temporary fix and more investigation will need to be done to properly understand why this behavior is erratic and why it only happens in some instances and not in others. More checks are also needed to ensure that proto files only have imports that are used in the proto file before they are merged. Besides build fixes, this PR also; - Includes a fix for the `buildifier_download.sh` which previously did not work for MacOS users. - Introduces a `static_checks.sh` file to allow contributors to run all static checks locally before pushing the code to Git. - Adds a wiki entry in the Static-Analysis-Checks wiki. The entry contains instructions on how to run static checks locally with the static_checks.sh file. ## Essential Checklist <!-- Please tick the relevant boxes by putting an "x" in them. --> - [x] 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: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). --------- Co-authored-by: Kenneth Murerwa <kkmurerwa@Kenneths-MacBook-Air.local> Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com>

    @kkmurerwa kkmurerwa committed Aug 3, 2023
  • Fix part of #3690 : Wiki update - How to run static CI check locally (#5108) <!-- READ ME FIRST: Please fill in the explanation section below and check off every point from the Essential Checklist! --> ## Explanation Fix part of #3690 : Wiki Update - How to run static CI check locally <!-- - Explain what your PR does. If this PR fixes an existing bug, please include - "Fixes #bugnum:" in the explanation so that GitHub can auto-close the issue - when this PR is merged. --> ## Essential Checklist <!-- Please tick the relevant boxes by putting an "x" in them. --> - [x] 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](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only <!-- Delete these section if this PR does not include UI-related changes. --> If your PR includes UI-related changes, then: - Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes - For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines)) - Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide)) - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

    @MohitGupta121 MohitGupta121 committed Jul 28, 2023
  • How to use GitHub’s search feature to search the wiki (#4961) <!-- READ ME FIRST: Please fill in the explanation section below and check off every point from the Essential Checklist! --> ## Explanation How to use GitHub’s search feature to search the wiki - With this contributor easily search and navigate to desire wiki page. - Added TOC in all wikis pages. <!-- - Explain what your PR does. If this PR fixes an existing bug, please include - "Fixes #bugnum:" in the explanation so that GitHub can auto-close the issue - when this PR is merged. --> ## Essential Checklist <!-- Please tick the relevant boxes by putting an "x" in them. --> - [x] 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: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only <!-- Delete these section if this PR does not include UI-related changes. --> If your PR includes UI-related changes, then: - Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes - For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines)) - Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide)) - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

    @MohitGupta121 MohitGupta121 committed May 9, 2023
  • Fix #4622: Android Wiki migration (#4893) <!-- READ ME FIRST: Please fill in the explanation section below and check off every point from the Essential Checklist! --> ## Explanation <!-- - Explain what your PR does. If this PR fixes an existing bug, please include - "Fixes #bugnum:" in the explanation so that GitHub can auto-close the issue - when this PR is merged. --> - Fixes #4622 - The PR does the following: This PR moves the android wiki into the oppia-andoird codebase allowing contributors to make PRs to the wiki along with changes to the codebase itself. This PR also introduces a GitHub Action (Runs when a PR is merged into develop and when a user makes change to the wiki via the web interface) that copies the content of the wiki folder along with the git history of the wiki folder and deploys it to the oppia-android wiki repository. The steps followed to achieve this wiki migration are present here: [Oppia Android Wiki Migration V2 (1).pdf](https://github.com/oppia/oppia-android/files/10921370/Oppia.Android.Wiki.Migration.V2.1.pdf) ### Note: Please ensure GitHub actions have write permissions. Please follow these instructions to enable it: [GrantingReadandWritePermissionsonGitHub_PDF.pdf](https://github.com/oppia/oppia-android/files/10919834/GrantingReadandWritePermissionsonGitHub_PDF.pdf) ## Essential Checklist <!-- Please tick the relevant boxes by putting an "x" in them. --> - [x] 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: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only <!-- Delete these section if this PR does not include UI-related changes. --> If your PR includes UI-related changes, then: - Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes - For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines)) - Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-(A11y)-Guide)) - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing --------- Co-authored-by: Rajat Talesra <rajattalesra4914@gmail.com> Co-authored-by: Ben Henning <henning.benmax@gmail.com> Co-authored-by: Akshay Nandwana <akshaynandwana001@gmail.com> Co-authored-by: Srikanth K B <srikanth.kadaba@gmail.com> Co-authored-by: Jan <fsharpasharp@users.noreply.github.com> Co-authored-by: Abhay Garg <54636525+prayutsu@users.noreply.github.com> Co-authored-by: Arjun Gupta <arjupta.90@gmail.com> Co-authored-by: Sparsh Agrawal <55937724+Sparsh1212@users.noreply.github.com> Co-authored-by: Farees Hussain <fareezzhussain@gmail.com> Co-authored-by: Sarthak Agarwal <agarwal.sarthak262012@gmail.com> Co-authored-by: Sean Lip <sean@seanlip.org> Co-authored-by: Apoorv Srivastava <53645584+MaskedCarrot@users.noreply.github.com> Co-authored-by: Vinita Murthi <murthi.vinita@gmail.com> Co-authored-by: Ankita Saxena <ankitasonu24@gmail.com> Co-authored-by: Vojtěch Jelínek <vojtech.jelinek@hey.com> Co-authored-by: Ben Henning <ben@oppia.org>

    @gp201 gp201 committed Mar 17, 2023