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

Update QA Checklists for Pull Requests #9268

Closed
clevinson opened this issue May 5, 2021 · 10 comments
Closed

Update QA Checklists for Pull Requests #9268

clevinson opened this issue May 5, 2021 · 10 comments
Assignees

Comments

@clevinson
Copy link
Contributor

Today in Regen's dev call we discussed that we should update the author checklists for PRs, and additionally add a new set of checklists for reviewers.

@ryanchristo
Copy link
Contributor

I spent some time reviewing the contributing docs today and looking through past pull requests. It would be nice to further discuss the current pain points and the desired outcome on our next call.

As a general observation, I like how we are currently linking some of the items to CONTRIBUTING.md. It might be nice to build on this, linking each item to more information in CONTRIBUTING.md while keeping each item relatively short.

@clevinson
Copy link
Contributor Author

Most of this is resolved #9342, only followup is to potentially add ability to tag specific folks in reviews.

@amaury1093
Copy link
Contributor

@ryanchristo can we close this since #9472 is merged?

@ryanchristo
Copy link
Contributor

I wasn't sure if we still needed to address #9342 (comment), so I left the issue open. Maybe we can close the issue, further discuss reviewers with specific roles on one of our team calls, and then open a new issue if we need to make changes. Thoughts?

cc @AmauryM @aaronc @clevinson

@aaronc
Copy link
Member

aaronc commented Jun 14, 2021

The one think I think we should do is require check boxes to be checked off in CI. I still see people aren't really using the checklists...

@ryanchristo
Copy link
Contributor

The one think I think we should do is require check boxes to be checked off in CI.

This task list completed application can be installed by an administrator for free.

@aaronc
Copy link
Member

aaronc commented Jun 14, 2021

The one think I think we should do is require check boxes to be checked off in CI.

This task list completed application can be installed by an administrator for free.

@okwme @alexanderbez what do you think? I'm not an org admin.

@ryanchristo did you find any good options we could install as an action?

@aaronc
Copy link
Member

aaronc commented Jun 14, 2021

@ryanchristo I don't see an option to choose another template... is that because we still have .github/PULL_REQUEST_TEMPLATE.md in the root? I don't think change a URL is a good option. If there isn't an easy way to select a template, I think we'll need to simplify the default template and then have some contextual QA checklists that pop up in comments.

@ryanchristo
Copy link
Contributor

@ryanchristo did you find any good options we could install as an action?

The first option was what I had from our call notes. It also looks like there is another option that can be configured as an action. I can open a pull request that adds it as an action but this would not apply to pull request comments.

I don't see an option to choose another template... is that because we still have .github/PULL_REQUEST_TEMPLATE.md in the root? I don't think change a URL is a good option.

Unfortunately there is no UI to choose another pull request template as there is for issues. GitHub requires the author to manually add the parameter to the URL. I thought using the fix-feature-refactor template as the default template and leaving a comment that explains how to access the other templates was the best solution at the time. See #9342 (comment).

@clevinson
Copy link
Contributor Author

clevinson commented Jun 15, 2021

If there isn't an easy way to select a template, I think we'll need to simplify the default template and then have some contextual QA checklists that pop up in comments.

@aaronc Given that the UX for selecting different templates is pretty lacking, I think we should probably take this approach using comments in PR action or just GitHub scripts as outlined in this repo.

For now, how about we try to get more serious about everyone making use of the checklists in the current default template , and then let's prioritize a v2 of this (including GitHub actions w/ comments etc.) separately?

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

No branches or pull requests

5 participants