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

Automate some of the PR things #1069

Open
gregsdennis opened this issue Feb 1, 2021 · 6 comments · May be fixed by #1072
Open

Automate some of the PR things #1069

gregsdennis opened this issue Feb 1, 2021 · 6 comments · May be fixed by #1072

Comments

@gregsdennis
Copy link
Member

@Relequestual requested that some things be automated. One of those things included ensuring that an issue be associated with PRs. This is the issue associated with the PR that adds automation to ensure that issues are associated to PRs.

@jdesrosiers
Copy link
Member

jdesrosiers commented Feb 1, 2021

@karenetheridge: “all PRs should have an associated issue” — I mostly agree, except for draft PRs - as sometimes, the best way to express an idea is to show the patch for it

@gregsdennis When you say you want to automate this, what does that mean? Will there be a way to skip this requirement when it's appropriate? For example, a bot comment on a PR that says "Hey! Did you forget to link an issue" would be great. It's a nudge that keeps us honest, allows us to ignore when appropriate, and takes the burden off of us to respond to new contributors with such a message. On the other hand, not allowing a PR to be created or auto-closing or blocking merge or blocking discussion would make the process unnecessarily rigid IMO.

@gregsdennis
Copy link
Member Author

@jdesrosiers please see the PR. It puts a checklist on the opening comment, then ensures that all items are checked.

@jdesrosiers
Copy link
Member

I just looked at it, but I'm afraid I'm still confused. I don't have experience with Github Actions and looking through the configuration wasn't helpful without a baseline understanding.

What does "ensures that all items are checked" mean? Does that mean merge is blocked until all items are checked? Does the checkbox ensure that you can't check it if there isn't an issue linked? Or, is it just a nudge and we can check it if we have a good reason to not need an issue?

@gregsdennis
Copy link
Member Author

gregsdennis commented Feb 1, 2021

Does that mean merge is blocked until all items are checked?

The build will fail, but it doesn't completely block merging, just like any other build process.

Does the checkbox ensure that you can't check it if there isn't an issue linked? Or, is it just a nudge and we can check it if we have a good reason to not need an issue?

It's superficial. There is no actual check. The action adds the checklist then checks to ensure all items are checked. That's as deep as it gets.

@jdesrosiers
Copy link
Member

I like checklists, but I'm not sure that having an associated issue is a good thing to include in a checklist if it's not a hard requirement. It leaves things in a weird state when skipping is appropriate. Either you have to check to box and lie or ignore the build error. Lying is a better option than ignoring a build error, but it's not great.

@gregsdennis
Copy link
Member Author

gregsdennis commented Feb 8, 2021

The comment can be edited, too, to remove the check item...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants