Pull Request descriptions

[kodebach]

Our template for PR descriptions only contains various checklists. It doesn't encourage people to write an actual description of the changes contained in the PR. Sometimes the title is good enough, but there are already quite a few PRs were it is hard to know what it is about, without looking at the changed files.

@markus2330 @mpranj I'd simply add a section "Changes" to the top of the PR template:

## Changes

Describe what changes you implemented in this PR here.
The description should be more technical than the release notes.
It should give a quick overview so other developers/reviewers know what problem PR solves.

## Basics

These points need to be fulfilled for every PR:
[...]

[mpranj]

I agree that this is a bit cumbersome. I usually resort to the _preparation_next_release.md, but often the changes are not properly documented there either.

The other problem is how to avoid duplicate work, i.e. copypasting from _preparation_next_release.md to ## Changes back and forth until the PR is done.

[kodebach]

Oftentimes the PR description and the Release Notes won't be exactly the same, since the former is developer-facing while the latter is user-facing. However, I see your point.

Maybe we should keep the PR description more general:

## Changes

Provide a short (1-2 sentences) description of the problem the PR solves (if applicable reference issues).
The description should be targeted at Elektra developers and potential reviewers.
User-facing changes, API changes and implementation details should be permanently documented in the release notes or other documentation files instead.

For example:
> Reimplements the tools library in C, so it can be used in non-C++ code.

## Basics

These points need to be fulfilled for every PR:
[...]

In the example above, the release notes and other documentation would contain details about the new API, etc. This extra documentation needs to evolve with the PR, but the description in the PR itself is general enough that it only needs to change, when the scope of the PR changes. For example, if kdb merge is also updated in the same PR, we would add:

Also contains a rewrite of kdb merge using the new library.

[markus2330]

I don't think we should use the PR description for anything but the checklist as this information gets lost on merges. As already noted the release notes should be user-facing, answering: "What was done?" The "How was it done?" sometimes can be found in the git commit messages. But most of the time I am happy to find useful text in the release notes.

I don't think anyone wrote bad release notes/commit messages on purpose. If they are asked to write PR description, a similar text would be there. I agree with @mpranj that it often would end up containing duplicated information, not helping anybody.

[kodebach]

I don't think we should use the PR description for anything but the checklist as this information gets lost on merges.

I totally agree. However, 1-2 sentences that tell us what to expect without having to click through to the release notes would be nice. I tried this in #4239. It is a slight duplication, but I think the summary makes it easier to know what the intention was.

[kodebach]

Separately from the potential "Changes", I think we should utilise comments in the template:

## Checklist

<!--- Check relevant points but **please do not remove entries**.
For docu fixes, spell checking, and similar none of these points below
need to be checked. -->

- [ ] I added unit tests for my code
- [ ] I fully described what my PR does in the documentation
      (not in the PR description)
- [ ] I fixed all affected documentation
- [ ] I added code comments, logging, and assertions as appropriate (see [Coding Guidelines](https://master.libelektra.org/doc/CODING.md))
- [ ] I updated all meta data (e.g. README.md of plugins and [METADATA.ini](https://master.libelektra.org/doc/METADATA.ini))
- [ ] I mentioned every code not directly written by me in [THIRD-PARTY-LICENSES](doc/THIRD-PARTY-LICENSES)

This would render as the following (between the lines)

---

Checklist

• I added unit tests for my code
• I fully described what my PR does in the documentation
  (not in the PR description)
• I fixed all affected documentation
• I added code comments, logging, and assertions as appropriate (see Coding Guidelines)
• I updated all meta data (e.g. README.md of plugins and METADATA.ini)
• I mentioned every code not directly written by me in THIRD-PARTY-LICENSES

---

Then only the actual checklists show and the PR description is much cleaner.

[markus2330]

Yes, I agree! Using comments is a very good idea.