Create checklist for PRs and issues

[fingolfin]

During a recent discussion, there was interest in

have a checklist for PRs and Issues (albeit not one that is given as a github template!)

and I volunteered to create one. But now I wonder the following:

1. What should really be in it?
2. Where to put it then? In the wiki?
3. How will people find it there?
4. Does it really have any value? Won't it be ignored like the READMEs or most of the other Wiki material?
5. What is actually bad about putting it into a GitHub PR template, as described here ? If it's there, at least everybody who opens a new PR will see it; if they ignore it, it's their choice.

I'd appreciate comments, esp. by the people who were at the discussion (@alex-konovalov @ChrisJefferson @frankluebeck @markuspf @sebasguts @stevelinton @ThomasBreuer) but of course also by everybody else.

As to the content of the checklist, here is a first draft (the label names are currently being changed by me, but the gist should be clear enough):

checklist draft

When submitting a new PR, please take care of the following:

1. Add relevant labels:

   1. either release notes: not needed or release notes: to be added;
   2. at least one of the labels bug or enhancement or new feature;
   3. for changes meant to be backported to stable-4.9, add the backport-to-4.9 label;
   4. consider adding any of the labels build system, documentation, kernel, library, tests;

2. If your code contains kernel C code, run clang-format on it; the simplest way is to use git clang-format, e.g. like this (don't forget to commit the resulting changes)

   git clang-format $(git merge-base master)
   

Thoughts on the checklist draft:

• we'd have to update this to say backport-to-4.10 in the future. Or we rename that label to backport-to-stable (then some queries on the Wiki using it need to be updated).
• any other things you'd like to see mentioned?

[ChrisJefferson]

I think that looks like a good guide. My temptation would be stick with backport-to-4.9 for now, we can always move to just backport-to-stable when we need to change for 4.10 anyway (or realise we really care about the difference at that point, if for some reason we do).

I would be tempted to put this into the PR box if we can (I realise we used to have a big block of text for issues, which I worried would put people off, but I think expecting people submitting PRs to think about what they are doing is much more reasonable).

One thing I sometimes worry about / find arkward is running clang-format on 2 or 3 commits, once I have realised I've forgotten. If we were happy with a single clang-format commit at the end of a PR, that would certainly make me happy as it would make my life easier :)

[markuspf]

I remember pull-request templates to be awkward and getting in the way, but pull-requests were much less cumbersome and complicated then.

Now, forgetting things on a checklist is even more awkward and irritating, because I have to go back and change things/rebase/etc, but to be honest, a PR template will not change that (for me), I am useless like that.

My idea was that I have a printed off piece of paper on my desk that I have to thumb through. Obviously that is something I can do myself and share (or not).

I'd like a mention of "did you consider adding tests".

[olexandr-konovalov]

@fingolfin thank you. I don't see a problem with restoring a PR template. Even if it will be deleted, it draws attention to a whole variety of labels (which sometimes we may do not even suspect to exist, because there are no notifications for label change), and provides a handy reminder for clang-format. I think the more rarely PR authors are submitting PRs, the more useful such a reminder could be.

One should mention that "Add relevant labels" only applies to those having rights to add labels, to avoid confusing PR authors who are not in the gap-system organisation.

[olexandr-konovalov]

A PR template may have a checklist not only for a submitter, but also, importantly, for a reviewer. E.g. what to check, what to do with labels, etc.