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.

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

feat(ci): TODO GitHub issue checker #7642

Merged
merged 6 commits into from
Oct 15, 2023
Merged

feat(ci): TODO GitHub issue checker #7642

merged 6 commits into from
Oct 15, 2023

Conversation

clabby
Copy link
Member

@clabby clabby commented Oct 12, 2023

Overview

Adds a script to ops that greps through the codebase for TODO comments that reference GitHub issues. If any of the issues
referenced are closed, an error is thrown, and the script exits with a non-zero exit code and an error asking the user
to remove the stale TODO comment.

The script also has several options:

  • --verbose: Prints out warnings and a summary for issues that were either not found as well as TODO comments with invalid formats.
  • --strict: Fails if any TODO comments are found that fit an invalid format.

Accepted TODO formats

  • TODO(<issue_number>): <description> (Default org & repo: "ethereum-optimism/optimism")
  • TODO(repo#<issue_number>): <description> (Default org "ethereum-optimism")
  • TODO(org/repo#<issue_number>): <description>

Examples

Screenshot 2023-10-12 at 3 58 20 AM Screenshot 2023-10-12 at 3 58 38 AM

TODO

  • Get a PAT secret for the ethereum-optimism organization in order for CI to be able to reference issues in client-pod.

@clabby
Copy link
Member Author

clabby commented Oct 12, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

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

Nice. :)

@ajsutton
Copy link
Contributor

Somehow I lost the comment I was actually trying to add...

I'd suggest we run this only on a schedule and not with every PR. Otherwise closing an issue could prevent unrelated PRs from failing to merge which would be very disruptive. We can either add a slack notification on failures or possibly better, just reopen the issue. :)

@clabby clabby force-pushed the cl/todo-hoopty branch 2 times, most recently from c8a7b7e to f1f3143 Compare October 12, 2023 02:19
@clabby
Copy link
Member Author

clabby commented Oct 12, 2023

Somehow I lost the comment I was actually trying to add...

I'd suggest we run this only on a schedule and not with every PR. Otherwise closing an issue could prevent unrelated PRs from failing to merge which would be very disruptive. We can either add a slack notification on failures or possibly better, just reopen the issue. :)

Added this - great point 👍

@clabby clabby marked this pull request as ready for review October 12, 2023 02:39
@clabby clabby requested a review from a team as a code owner October 12, 2023 02:39
@clabby clabby requested a review from twoshark October 12, 2023 02:39
Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

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

LGTM. Can try to tighten up the bash mode for a bit more safety but not worth stressing about too much given this is just informational.

ops/scripts/todo-checker.sh Show resolved Hide resolved
@clabby clabby self-assigned this Oct 12, 2023
@clabby clabby added the M-do-not-merge Meta: Do not merge label Oct 12, 2023
@clabby
Copy link
Member Author

clabby commented Oct 12, 2023

Adding M-do-not-merge Meta: Do not merge . Need a GH PAT secret in CircleCI to read the private issues in the client-pod repo before we move everything into the public.

@sebastianst
Copy link
Member

@clabby I added a PAT that should have the right permissions to CCI's env var CI_TODO_CHECKER_PAT. Please give it a try.

@clabby clabby removed the M-do-not-merge Meta: Do not merge label Oct 13, 2023
@clabby clabby added this pull request to the merge queue Oct 13, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 13, 2023
@ajsutton ajsutton added this pull request to the merge queue Oct 15, 2023
Merged via the queue into develop with commit 56648d4 Oct 15, 2023
@ajsutton ajsutton deleted the cl/todo-hoopty branch October 15, 2023 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants