PULL_REQUEST_TEMPLATE.md: neon sha checkbox note

[vitorgalvao]

Next time I’ll change the checkbox to TICKING THIS CHECKBOX KILLS KITTENS.

I bet people will still click it without reading the instructions.

[reitermarkus]

This really has no end, has it?

[vitorgalvao]

This really has no end, has it?

I’m afraid not.

The initial PR had the bold message as an h3 instead. If this one doesn’t work, I’ll change it to that instead. After that, I’m not sure what to do anymore.

[vitorgalvao]

If we get to a point where the conclusion is that it’s impossible to make an acceptable checkbox, maybe we should just remove it. Then when someone makes a PR that require confirmation, we Request Changes with a snippet linking to the instructions.

[commitay]

If we get to a point where the conclusion is that it’s impossible to make an acceptable checkbox, maybe we should just remove it.

I'm in favour of removing it now.

[vitorgalvao]

I'm in favour of removing it now.

What’s your say on that, @reitermarkus?

Also, so I get an idea, how stressful is that checkbox for you both? I’ll say that to me the answer is very. It gets me genuinely rilled up inside when it’s misused (ticked when it should be but no confirmation provided; ticked when it shouldn’t; not ticked when it should) in a way that even surprises myself. I have no such reaction to the other checkboxes. I’m asking to understand if it’s just me.

[commitay]

It gets me genuinely rilled up inside when it’s misused (ticked when it should be but no confirmation provided; ticked when it shouldn’t; not ticked when it should) in a way that even surprises myself.

Same.

[reitermarkus]

I'm not in favour of not removing it. 😜

[vitorgalvao]

Removed. Just need to remember to fix the commit message when merging.

[commitay]

To do after merging:

• cask-repair template: cask-repair: remove sha256 message vitorgalvao/tiny-scripts#104
• run sync script

[miccal]

I am in favour of leaving it in and taking a leaf out of @MikeMcQuaid's philosophy of just immediately closing the PR with a blurb about not reading the instructions, as they do in the brew and homebrew-core repos.

[commitay]

I am in favour of leaving it in and taking a leaf out of @MikeMcQuaid's philosophy of just immediately closing the PR with a blurb about not reading the instructions, as they do in the brew and homebrew-core repos.

This works for them but not as well for us. A sha256 change means that the Cask is broken.

If we close the PR the Cask stays broken until another user opens a PR (or more likely, an issue) or a maintainer fixes it themselves.

[miccal]

I understand that, but the Cask will have to be fixed anyway if the sha256 change has not been properly verified, and closing the PR's and creating a new one that does it properly will hopefully get the point across.

[commitay]

#39703 (comment)
Then when someone makes a PR that require confirmation, we Request Changes with a snippet linking to the instructions.

---

closing the PR's and creating a new one that does it properly will hopefully get the point across.

Closing a PR for another PR assumes that the contributor will actually look at it?

[vitorgalvao]

In sum, I agree with @commitay.

philosophy of just immediately closing the PR with a blurb about not reading the instructions

I do that ruthlessly, but for issues. Then, if the error in the issue is enough clue of the problem, I go and fix it. I then go back to the original issue and link back to the new PR. This both avoids users complaining about the issue being closed and shows how easy it is to fix, as I usually can do it before they even have time to reply. My snippet for that is:

If you ignore this guide, your issue may be closed without review.

Also note from that document:

If the maintainer was wrong in closing your issue, please do reply stating why! Closing an issue does not mean the conversation is over. If the guides themselves were unclear, help us improve them! Open first an issue or pull request stating what you found confusing and only then your other issue.

But for PRs that’s harder to justify, and I’m not sure they do that in HB. Let’s look at which one of those says.

• If we auto-close on an issue with a wrongly filled template: “Please don’t be lazy. We worked hard on those guides and by not following them you’re saving yourself time and making us spend it”.
• If we auto-close on a PR: “You spent time on this but we’re not going to spend some ourselves explaining what’s missing”.

I have no quarrel with the first one, but I do with the second. In the first one the user is spitting in our face (yes, exaggeration) while on the second we’re spitting in theirs. I want us to lead by example.

With all that in mind, the sha256 verification process is a boring process that I don’t want to do have to do myself every time. Perhaps because of that the checkbox is a bigger amount of stress. I don’t want to just auto-close and go through the time-consuming process every time — I want users to learn it and do it.

I feel that if we auto-close PRs in those cases users will just think “I don’t care, then”. On the other hand, if we politely Request Changes pointing to instructions, they will follow it. I believe that because that’s what I do when requesting appcasts, and it always works out.

[miccal]

On the other hand, if we politely Request Changes pointing to instructions, they will follow it.

Fair enough.