* .github/PULL_REQUEST_TEMPLATE.md: Convert into checklist

[yantar92]

Checklist is generally easier to follow. Also, encourage contributions even when unsure about following conventions.

Related: #217.

[tarsius]

I'm going on a vacation for a week and will look at this when I get back.

[yantar92]

Sure. No problem. I am used to "wait one month" policy :)

[tarsius]

(Vacation cut short.)

I am not a fan of checklists in issues and pull-requests. As a user I often feel like being made to jump through hoops. As a maintainer I get the impression many users have a similar reaction.

I agree that is a good idea to instruct users about what is expected, and for them to follow those instructions; but IMO forms of any kind often fail to accomplish that.

As you suggest, a checklist is useful as a tool for contributors, allowing them to check they haven't forgotten something. As a proof to the maintainer that one hasn't skipped steps, it is less useful.

The checklist you present here is a somewhat random subset of all the things a contributor should consider. I think it is better to (1) provide all conventions in the form of a checklist, and (2) do so outside the pull-request template; like I have done at https://github.com/emacscollective/no-littering/compare/checklist?expand=1.

[tarsius]

What do you think about that approach, and do you have any suggestions how to further improve it?

[yantar92]

Jonas Bernoulli @.***> writes:

I am not a fan of checklists in issues and pull-requests. As a user I often feel like being made to jump through hoops. As a maintainer I get the impression many users have a similar reaction.

I do not have a strong preference to have checkboxes specifically. I mostly tried to make a short-ish outline of what needs to be double-checked.

The checklist you present here is a somewhat random subset of all the things a contributor should consider. I think it is better to (1) provide all conventions in the form of a checklist, and (2) do so outside the pull-request template; like I have done at https://github.com/emacscollective/no-littering/compare/checklist?expand=1.

Is there any particular reason why examples of commit messages are listed in the template itself? That was the main thing that concerned me. If commit message conventions are listed, why other conventions are not?

-- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/. Support Org development at https://liberapay.com/org-mode, or support my work at https://liberapay.com/yantar92

[tarsius]

I do not have a strong preference to have checkboxes specifically. I mostly tried to make a short-ish outline of what needs to be double-checked.

So what I have done doesn't accomplish this at all. It does use a checklist, but it isn't short-ish but the full list.

Still I think it is useful because it is an optional tool the contributor can chose use. Since it is kept out of the template they won't feel forced to use it, but can do so if they feel it will help them.

Is there any particular reason why examples of commit messages are listed in the template itself? That was the main thing that concerned me. If commit message conventions are listed, why other conventions are not?

(I assume we are talking about the "this file contains a sexp part", not "package-name: Theme variable".)

I was wondering the same and dropped it. Obviously I've dropped other things as well, but while I was on the fence about those, I was more certain that dropping this was the right thing to do.

Even when sentences like these are present, I still look at the package in question to verify that, for example, the package indeed only offers one path-option. I essentially repeat the work expected from the contributor, and these sentences don't help much with that.

But these sentences theoretically are useful when we later come to the conclusion that some package/variable was themed wrongly. In that case it would give us some insight into whether we misunderstood something about the package (e.g., if we claim a file contains a sexp, but it does not), or whether we forgot to consider some aspect (in which case it wouldn't be mentioned in the commit message). That too turned out to be not all that useful in practice, so it is okay if it is missing. It's still better if it is there, but no biggie if not; so it doesn't belong in the template, just the conventions.

---

In any case, I think what I have done, addresses your main concern: the wording that suggested "you better get this right", is gone.

Again, I have added that at a time when I was annoyed by a string of suboptimal pull-requests, and had forgotten all about it, and agree how that it has no positive effect.

[yantar92]

Jonas Bernoulli @.***> writes:

I do not have a strong preference to have checkboxes specifically. I mostly tried to make a short-ish outline of what needs to be double-checked.

So what I have done doesn't accomplish this at all. It does use a checklist, but it isn't short-ish but the full list.

Still I think it is useful because it is an optional tool the contributor can chose use. Since it is kept out of the template they won't feel forced to use it, but can do so if they feel it will help them.

I am sorry, but may you please clarify what you mean by "what I have done"?

Is there any particular reason why examples of commit messages are listed in the template itself? That was the main thing that concerned me. If commit message conventions are listed, why other conventions are not?

(I assume we are talking about the "this file contains a sexp part", not "package-name: Theme variable".)

Yes.

I was wondering the same and dropped it.

I am confused. May you please link to the commit you are talking about?

In any case, I think what I have done, addresses your main concern: the wording that suggested "you better get this right", is gone.

(I assume that you are referring to https://github.com/emacscollective/no-littering/commit/6e7b83475bd63db321c5e90814a8cf814c633b5e)

Not sure.

"Going forward contributors are expected to follow the conventions more closely from the get-go and to be explicit about their thought process"

still reads as "you better gets this right".

-- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/. Support Org development at https://liberapay.com/org-mode, or support my work at https://liberapay.com/yantar92

[tarsius]

I was referring to the checklist branch and linked to https://github.com/emacscollective/no-littering/compare/checklist?expand=1. You are probably confused by the fact that the previous pull-request is still being shown in the textbox. That's because that pull-request hasn't been merged yet. While preparing that pull-request, the old pull-request template is still being shown. Click on the last commit listed below ("Provide a checklist with actual check boxes") to see the new template.

Do you have any suggestions how to improve the little text left in the template and the new prelude added to the conventions, added in https://github.com/emacscollective/no-littering/commit/be33212db778391a1dd3ffd75bc88b7b7d5f044d?

[yantar92]

Jonas Bernoulli @.***> writes:

You are probably confused by the fact that the previous pull-request is still being shown in the textbox.

Yup. That's exactly what happened.

Do you have any suggestions how to improve the little text left in the template and the new prelude added to the conventions, added in https://github.com/emacscollective/no-littering/commit/be33212db778391a1dd3ffd75bc88b7b7d5f044d?

To make it easier to follow the conventions, we provide a html export of that file, which actually allows checking the check boxes:

"html export" sounds a bit confusing.

Maybe something like

To make it easier to follow the conventions, you can use interactive
checklist in https://emacsmirror.net/manual/no-littering.html

-- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/. Support Org development at https://liberapay.com/org-mode, or support my work at https://liberapay.com/yantar92

[tarsius]

I've now merged the ckecklist branch, taking your latest suggestion into account. Thanks for bringing this up!