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

Implement a lint check to ensure all TODOs contain a username within it #7010

Closed
lilithxxx opened this issue Jun 25, 2019 · 4 comments · Fixed by #7114
Closed

Implement a lint check to ensure all TODOs contain a username within it #7010

lilithxxx opened this issue Jun 25, 2019 · 4 comments · Fixed by #7114
Assignees
@teddymarchildon
Labels
good first issue

Comments

@lilithxxx
Copy link
Contributor

Implement a lint check to ensure all TODOs contain a username within it. There should be no naked TODOs in the codebase. For example:

correct format
TODO(username): Do something
incorrect format
TODO: Do something
@lilithxxx lilithxxx mentioned this issue Jun 25, 2019
Merged
8 tasks
@teddymarchildon
Copy link
Contributor

@lilithxxx I am happy to work on this if still needed! Is the best place to add a new lint check in scripts/pre_commit_linter ?

@seanlip
Copy link
Member

seanlip commented Jul 6, 2019

Hi @teddymarchildon, yes! The linter script is the right place.

I've assigned you to the issue, please go ahead :)

@teddymarchildon teddymarchildon mentioned this issue Jul 8, 2019
Merged
8 tasks
@brianrodri
Copy link
Contributor

brianrodri commented Jul 16, 2019 •
edited
Loading

As a side note, I think it'd be fine to allow github issue numbers in TODO statements as well (since they are much more stable). What do you think @seanlip @oppia/dev-workflow-team?

@seanlip
Copy link
Member

seanlip commented Jul 17, 2019

I think that is a great idea (e.g. TODO(#213)). In fact I would go further and suggest that ultimately every TODO should look like this, otherwise the TODOs will just languish. (But that's out of scope for this PR.)

seanlip pushed a commit that referenced this issue Jul 23, 2019
@teddymarchildon @seanlip
Fixes #7010: Adds a linting check that ensures all TODO comments have…
d89d68f 
… a username with them (#7114)

* Add linting function to check that TODO comments have a user

* Fix regex to match digits present in usernames

* Put the bad regex for todo comments in the global variables for TS, JS, and PY files

* Add regex to HTML bad patterns, and fix TODO comments with the codeowner of the file

* Remove unnecessary comment

* Shorten line lengths in comments

* Add a user to a TODO comment in the jobs.py file

* Fix TODO comment in extensions

* Enforce the @ sign in TODO comments over the whole project

* Delete files that had been deleted in develop

* Change lowercase todo to uppercase TODO in pre_commit_linter

* Checkout the package-lock file to develop branch

* Fix package-lock.json file

* Force package-lock.json to be develop version

* Update TODO comment format in a few new locations

* Revert regexp to omitting @ sign

* Fix TODO assignments

* Add a message for sean's todo

* Put the TODO regexp in its own structure, and enforce : and message

* Fix more TODO comment formats

* Remove unnecessary + sign from message
@DubeySandeep DubeySandeep mentioned this issue Jul 23, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@teddymarchildon teddymarchildon

Labels
good first issue
Projects
None yet
Milestone
No milestone
4 participants
@brianrodri @seanlip @teddymarchildon @lilithxxx