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.

Sign up for GitHub

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

Jump to bottom

Fixes: #3296: Add check to ensure all TODOs are correctly formatted and correspond to an open issue on Github #3508

Merged
merged 249 commits into from
Aug 16, 2021
Merged

Fixes: #3296: Add check to ensure all TODOs are correctly formatted and correspond to an open issue on Github #3508

merged 249 commits into from
Aug 16, 2021

Conversation

Sparsh1212
Copy link
Contributor

@Sparsh1212 Sparsh1212 commented Jul 20, 2021 •
edited

Explanation

Fixes #3296

Check list

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
  • The PR follows the style guide.
  • The PR does not contain any unnecessary auto-generated code from Android Studio.
  • The PR is made from a branch that's not called "develop".
  • The PR is made from a branch that is up-to-date with "develop".
  • The PR's branch is based on "develop" and not on any other branch.
  • The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

@Sparsh1212
Fix #3290: Add support for generic regex pattern matching
225a4fc
@Sparsh1212
Apply review suggestions for proto location
638895f
@Sparsh1212
Delete main directory
e3fd706
@Sparsh1212
Implment script logic and tests
b25e776
@Sparsh1212
CI setup part 1
d44e015
@Sparsh1212
syntax fix in yaml file
de1d38f
@Sparsh1212
import nits in dummy file
173fe53
@Sparsh1212
Use regex patterns from script in script test
618e380
@Sparsh1212
Make PR suggestions into effect
e3830ac
@Sparsh1212
Make PR suggestions into effect
bb93c0a
@Sparsh1212
Make PR suggestions into effect
f7342c1
@Sparsh1212
Improve naming of script_assets variables
51e593b
@Sparsh1212
Make PR suggestions into effect
0d17657
@Sparsh1212
Make PR suggestions
16bb041
@Sparsh1212
Make PR suggestions
325f28c
@Sparsh1212
Revamp testing approach
a3f8ea6
@Sparsh1212
Merge branch 'develop' into regex-checks
b1970eb
@Sparsh1212
Fix #3291: Add check for XML syntax correctness
51e64aa
@Sparsh1212
nit fixes
f440a33
@Sparsh1212
Merge branch 'regex-checks' into xml-syntax-check
255773d
@Sparsh1212
Introduce XML syntax check in static checks workflow
7b2a178
@Sparsh1212
Fix #3292: Add check for test files presence for prod files
14c5f6f
@Sparsh1212
Make nit suggestions
163310e
@Sparsh1212
Merge branch 'regex-checks' into xml-syntax-check
6131c1d
@Sparsh1212
Make nit suggestions
9cb5fc5
@Sparsh1212
Merge branch 'xml-syntax-check' into test-file-check
4120775
@Sparsh1212
Make nit suggestions
8c98cf2
@Sparsh1212
Fix #3300: Add check for accessibility labels for activities
7f1762c
@Sparsh1212
Add CI check for label presence for activities
5229ac9
@Sparsh1212
Add ScriptConstants file to exemptions test list
7e1d24a
BenHenning
BenHenning reviewed Aug 13, 2021
View reviewed changes
Copy link
Sponsor Member

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

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

Thanks @Sparsh1212. The PR is look good to me, just had a few more comments--PTAL.

scripts/src/java/org/oppia/android/scripts/common/RepositoryFile.kt Show resolved Hide resolved
scripts/src/java/org/oppia/android/scripts/todo/TodoCollector.kt Outdated Show resolved Hide resolved
scripts/src/java/org/oppia/android/scripts/todo/TodoOpenCheck.kt Outdated Show resolved Hide resolved
scripts/src/java/org/oppia/android/scripts/todo/TodoOpenCheck.kt Outdated Show resolved Hide resolved
scripts/src/java/org/oppia/android/scripts/todo/TodoOpenCheck.kt Show resolved Hide resolved
scripts/src/java/org/oppia/android/scripts/todo/TodoOpenCheck.kt Outdated Show resolved Hide resolved
scripts/src/java/org/oppia/android/scripts/todo/TodoOpenCheck.kt Outdated Show resolved Hide resolved
scripts/src/javatests/org/oppia/android/scripts/todo/TodoCollectorTest.kt Show resolved Hide resolved
scripts/src/javatests/org/oppia/android/scripts/todo/TodoCollectorTest.kt Show resolved Hide resolved
@BenHenning BenHenning assigned Sparsh1212 and unassigned BenHenning Aug 13, 2021
@Sparsh1212 Sparsh1212 assigned BenHenning and unassigned Sparsh1212 Aug 13, 2021
BenHenning
BenHenning requested changes Aug 14, 2021
View reviewed changes
Copy link
Sponsor Member

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

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

Thanks @Sparsh1212. Had a few more comments.

scripts/src/javatests/org/oppia/android/scripts/common/RepositoryFileTest.kt Outdated Show resolved Hide resolved
scripts/src/javatests/org/oppia/android/scripts/todo/TodoCollectorTest.kt Show resolved Hide resolved
scripts/src/java/org/oppia/android/scripts/todo/TodoOpenCheck.kt Show resolved Hide resolved
scripts/src/javatests/org/oppia/android/scripts/todo/TodoCollectorTest.kt Show resolved Hide resolved
@BenHenning BenHenning assigned Sparsh1212 and unassigned BenHenning Aug 14, 2021
@Sparsh1212
Copy link
Contributor Author

Thanks @Sparsh1212. Had a few more comments.

Done. PTAL.

@Sparsh1212 Sparsh1212 assigned BenHenning and unassigned Sparsh1212 Aug 14, 2021
@Sparsh1212 Sparsh1212 removed their assignment Aug 14, 2021
BenHenning
BenHenning approved these changes Aug 16, 2021
View reviewed changes
Copy link
Sponsor Member

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

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

Thanks @Sparsh1212. LGTM.

@BenHenning BenHenning requested a review from rt4914 August 16, 2021 18:03
@BenHenning BenHenning assigned rt4914 and unassigned BenHenning Aug 16, 2021
@BenHenning
Copy link
Sponsor Member

@rt4914 I think you need to review a trivial change in an XML file to unblock this PR.

@oppiabot
Copy link

oppiabot bot commented Aug 16, 2021

Assigning @anandwana001 for code owner reviews. Thanks!

@BenHenning BenHenning removed the request for review from anandwana001 August 16, 2021 18:06
rt4914
rt4914 approved these changes Aug 16, 2021
View reviewed changes
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

Approved for code owner. Thanks.

@rt4914 rt4914 removed their assignment Aug 16, 2021
@BenHenning BenHenning merged commit aff226b into develop Aug 16, 2021
@BenHenning BenHenning deleted the todo-check branch August 16, 2021 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rt4914 rt4914 rt4914 approved these changes

@vinitamurthi vinitamurthi vinitamurthi approved these changes

@BenHenning BenHenning BenHenning approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create a Kotlin script to verify that all the TODOs present in the codebase corresponds to an open issue
6 participants
@Sparsh1212 @BenHenning @jcqli @rt4914 @vinitamurthi @anandwana001