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

ci: add pr task completed checker #9528

Closed
wants to merge 1 commit into from

Conversation

ryanchristo
Copy link
Collaborator

@ryanchristo ryanchristo commented Jun 16, 2021

Description

Ref: #9268

This pull request adds a GitHub action that requires all task list items to be completed before merge.

See task-completed-checker-action for more details.


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed - "Task Completed Checker" not passing ; )

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed all author checklist items have been addressed
  • confirmed that this PR does not change production code

Copy link
Member

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

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

not sure the value of making this process so strict. A contributor can still delete the whole file and ci would pass.

@ryanchristo
Copy link
Collaborator Author

A contributor can still delete the whole file and ci would pass.

This is true.

not sure the value of making this process so strict.

Very few people were using the checklists and we are making an effort to improve our QA process. Adding this action to require all checklist items to be completed was one recommendation - see #9268 (comment). Any other ideas?

@tac0turtle
Copy link
Member

Any other ideas?

oh, I think this is the best solution. I haven't put much thought to this. Human intervention is the best, this is the next best thing IMO.

@amaury1093
Copy link
Contributor

amaury1093 commented Jun 18, 2021

I think having all items checked is too strict. I just reviewed #9533, but there are still a couple unchecked ones, which imo are justified. Another example is #9522, where Tyler striked out unrelevant items.

If we merge this, let's at least not make it required at the beginning.

@ryanchristo
Copy link
Collaborator Author

I think having all items checked is too strict. I just reviewed #9533, but there are still a couple unchecked ones, which imo are justified. Another example is #9522, where Tyler striked out unrelevant items.

If we merge this, let's at least not make it required at the beginning.

Yea, the general consensus seems to be this is too strict. If its not required, I feel like it would just be adding noise. I'm going to close this for now and move the issue to the backlog. Let's work with what we have at the moment.

@ryanchristo ryanchristo deleted the ryan/9268-checklist-action branch August 12, 2021 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants