Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

Commit

Permalink
Merge pull request #4562 from markus2330/markus/decisions
Browse files Browse the repository at this point in the history
decisions: decide process, improve manpages
  • Loading branch information
markus2330 authored Nov 1, 2022
2 parents 61418ca + 99cb078 commit fb593f5
Show file tree
Hide file tree
Showing 8 changed files with 191 additions and 72 deletions.
5 changes: 3 additions & 2 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,9 @@ 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 (see [Documentation Guidelines](https://master.libelektra.org/doc/contrib/documentation.md))
- [ ] I added code comments, logging, and assertions as appropriate (see [Coding Guidelines](https://master.libelektra.org/doc/CODING.md))
- [ ] I fixed all affected documentation (see [Documentation Guidelines](https://www.libelektra.org/devgettingstarted/documentation))
- [ ] I fixed all affected decisions (see [Decision Process](https://www.libelektra.org/decisions/decision-process))
- [ ] I added code comments, logging, and assertions as appropriate (see [Coding Guidelines](https://www.libelektra.org/devgettingstarted/coding))
- [ ] 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](/.reuse/dep5) not directly written by me in [reuse syntax](https://reuse.software/)

Expand Down
9 changes: 9 additions & 0 deletions doc/decisions/EXPLANATIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,21 @@ This can be:
## Related Decisions

This section has links to other decisions with description what the relation is.
One-side relations are allowed, not every decision must link back.
Decisions that give constraints must be listed in "Constraints" above.

> Note:
> Sometimes the best solution is only understood if the relation between decisions becomes clear.
> Make sure that everything that requires updates to a decision, is listed as "Constraints" or "Assumptions".
## Notes

Here is a full list of off-line discussions, issue trackers, PRs etc. related to this decision.
Preferable it is linked, but if it is not possible, it can also be in full-text here.
If particular information is important and not present in any sections above, please quote it here.

Any incomplete and unexplored idea/opinion, which is not complete enough to be in "Considered Alternatives", can be written here.
For example, if it is obvious that the idea does not even solve the problem.
Unlike the main decisions and considered alternatives, text in the notes does not need rationale.

Furthermore, the author, acknowledgements, dates etc. can be written here.
4 changes: 2 additions & 2 deletions doc/decisions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ Before you write your first decision, read [Decision Process](decision_process.m

## Decided

- [Decision Process](decision_process.md)
- [Capabilities](capabilities.md)
- [Make elektraMalloc et al. private](elektra_malloc.md)
- [Elektra Prefix](elektra_prefix.md)
Expand All @@ -68,13 +69,12 @@ Before you write your first decision, read [Decision Process](decision_process.m
- [Error Handling](error_handling.md)
- [Spec Expressiveness](spec_expressiveness.md)
- [Metadata in Spec Namespace](spec_metadata.md)
- [Decision Process](decision_process.md).

## Drafts

- [Notifications](notifications.md)
- [Header File Structure](header_file_structure.md)
- [Manpages](manpages.md)
- [Man Pages](man_pages.md)

## Delayed

Expand Down
8 changes: 5 additions & 3 deletions doc/decisions/TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@

## Considered Alternatives

1.
2.
3.
### Alternative A

### Alternative B

### Alternative C

## Decision

Expand Down
109 changes: 84 additions & 25 deletions doc/decisions/decision_process.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,32 @@
## Problem

Simply discussing in an issue and then implementing a solution is okay for non-substantial changes.
Substantial decisions must be made in a transparent and participative way.
Substantial decisions, however, must be made in a transparent and participative way.
Discussing fundamental problems in forum-like threads showed to be repetitive and ineffective.
Decisions by supervisors are undemocratic and decisions in meetings are nontransparent for the outside world.

### Terminology

- `Decision`:
A text file which contains the content as [explained here](EXPLANATIONS.md).
- `Decision PR`:
A pull request that contains changes for decisions.
- `Decision author`:
Is the person who creates the decision PR.
- `Decision reviewer`:
Is the person who reviews the decision PR.

### Main Purpose

- of decisions is to have clear descriptions of technical problems and solutions.
- of the decision process is to get a common understanding of the problems and the impacts of possible solutions.

## Constraints

- All relevant information about decisions must be within Elektra's repository.
- All decisions must go through at least two review rounds, with a merge in between.
- At least two people need to be in favor of the decision in both rounds.
- [Documentation guidelines](/doc/contrib/documentation.md) apply.
- At least two people need to approve the decision in each round.
- [Documentation guidelines](../contrib/documentation.md) apply.
- During the decision process, the PRs constantly get updated:
- Make changes as new commits to the pull request.
- Questions in the PRs are answered by:
Expand All @@ -22,26 +40,54 @@ Substantial decisions must be made in a transparent and participative way.
Committing a suggestion directly on GitHub does this automatically.
- As generally recommended in Elektra, do not squash commits after they are visible on the pull request.
- Rebase only if the decision was already accepted and has a merge conflict.
- For reviewers:
- For decision reviewers:
- Prefer to directly give suggestions how to change sentences.
- General questions should be asked in the root of "Conversation" and not at random sentences in the review.
- General questions should be asked in the root of "Conversation" and not at vaguely related sentences in the review.
- Decision PRs only modify a _single_ decision.
Small exceptions like backlinks from other decisions are okay, though.
- Changes not changing the decision step or the direction of the decision are not decision PRs.
- The person merging the decision PR must be someone other than the person that created the decision.
There is no claim that decisions contain everything that was said.
In particular corrections of wrong decision text is, if at all, only visible via git history.
Rather it is important that decisions:
- contain everything relevant, and
- what is written is technically correct.
- Usually no decisions are needed for libraries, plugins, tools or bindings if:
- they are similar to already existing modules (e.g. yet another checker plugin)
- if they reimplement some existing module (e.g. reimplementation in other programming languages)
- Reviewers are only required to read the files in the PR.
They may ignore the discussions surrounding the decision.
Reviews should focus on the "Problem" section first, and only when that is agreed upon focus in the other parts.
It is encouraged that at least one decision reviewer provides a review _without_ participating in the discussion.
This ensures that there aren't any unintentional shared assumptions between discussion participants.

## Assumptions

- People want to be informed about or even participate in what Elektra looks like in the future.
- People writing or reviewing decisions want Elektra to improve, so they also want to accept (acceptable) decisions.
In general they also want change if it brings Elektra towards its [goals](/doc/GOALS.md) (but doesn't violate Elektra's stability guarantees).
- We will always be able to reach an consensus.
- Decision authors have some scientific background and want decisions based on science, and not only on opinions.
- If assumptions, including this ones written here, are broken, decisions will be redone.
- Decision authors and reviewers want Elektra to improve, so they also want to accept (acceptable) decisions.
In general people want change if it brings Elektra towards its [goals](/doc/GOALS.md).
- All decision reviewers want to help the decision authors to write a good decision.
- Decision authors are the main force behind a decision and possibly also of specific solutions.
Nevertheless they don't want to avoid other solutions, are open to arguments/facts/etc. and incorporate all input of decision PR fairly.
- We will always be able to reach an consensus even if it requires that the core or plugins get multiple implementations.
We don't need a vote (besides the approved review) or a benevolent dictatorship.
- Unlike the Rust Decision process, decisions in Elektra do not have a disadvantage if they were flawed in early stages.
Only the end results counts.
- Different to initiatives like Rust, most contributors in Elektra are not experts in configuration management or programming languages.
So we do not expect that a clear problem or solution is in the decision writer's mind beforehand.
Instead the decision process is a supported learning process.
- People focus on getting the best solutions and not to wish for the impossible.
- People creating decision PRs have strong motives to also finish it.
They take extra effort on them to be clear about the problem and find the best solution.

## Considered Alternatives

- Issues like https://issues.libelektra.org/4521
- GitHub discussions
- Votings
- The maintainer decides
- PEPs: https://peps.python.org
- RFCs: https://www.ietf.org/standards/rfcs/
- Change requests: https://en.wikipedia.org/wiki/Change_request
Expand All @@ -61,35 +107,40 @@ We base our decision process and template on:
- [ADR](https://adr.github.io/), and
- [RFCs in rust-lang](https://github.com/rust-lang/rfcs).

Following subsections describe all steps a decision might run through.
Only two of them are mandatory.

We use the template [TEMPLATE.md](TEMPLATE.md).
Explanations of the template are in [EXPLANATIONS.md](EXPLANATIONS.md).

Following subsections describe all steps a decision might run through.
Each step requires two reviews and the merging of the decision PR.

In each step we directly update the decision text with the different opinions.
Discussions should focus on the decision text so that the text evolves with the opinions.

### Drafts

The first step is to create a PR with:

- **one** decision, where at least the "Problem" and "Considered Alternatives" are filled out.
- **one** decision, where at least the "Problem" is filled out and "Decision", "Rationale" and "Implications" are **not** yet filled out.
- a link from [README.md](README.md) from the "Drafts" section to this decision.
- optional backlinks from related decisions.

> At least the problem must be clear to everyone involved before the decision can leave the "Drafts" status.
> It must be so clear that everyone would be able to write a test case that shows if a solution fixes the problem.
### In Discussion

This step is mandatory.
> This step is mandatory.
Here you must ensure:

- problem, constraint and assumptions are well-explained and sound
- consistency with other decisions
- links from/to related decisions are created
- problem, constraint and assumptions are fully described and sound
- there are several considered alternatives, each with rationale and implication
- decision, rationale and implications is **not** yet filled out if there are people arguing for different options (to keep the discussion unbiased)
- "Decision", "Rationale" and "Implications" are **not** yet filled out if there are people arguing for different options

Here "the decision" should not only have one decision but should describe several solutions.
For each solution a proposal, rationale and implication should be given.

This step is finished when every reviewer approves.
Here the decision should not only have one decision but should describe several solutions.
For each solution a proposal, rationale and optionally implications should be given.

### In Progress

Expand All @@ -98,7 +149,9 @@ This step is finished when every reviewer approves.

### Decided

- decision, rationale and implication are now filled out and fixed according to the reviews
> This step is recommended.
- "Decision", "Rationale" and "Implications" are now filled out and fixed according to the reviews
- decisions of this status usually already have an implementation PR

### Partially Implemented
Expand All @@ -110,14 +163,12 @@ The "Implication" must clearly say how much of the decision is already implement

### Implemented

This step is mandatory.
> This step is mandatory.
- Here the details of the decisions are stripped from the decision and moved to the documentation.
- The documentation links to the decision.
- The decision links to the new documentation.

This step is finished when every reviewer approves.

### Rejected

Alternatively, decisions might be rejected (i.e. status quo wins).
Expand All @@ -129,23 +180,31 @@ These decision PRs are also merged for documentation purposes.
- The template makes sure important points are not forgotten.
- Every decision is by design in its own file with its own git history.
- PRs allow to better support the constraint that everything must be within Elektra's repository (also rejected PRs).
- "Decision", "Rationale" and "Implications" are filled out later to keep the discussion unbiased
- PRs allow to suggest changes and review individual sentences of the decision.
- Several "Related Decisions" are very important even if everyone agrees on one solution.
They allow reviewers and future readers of the decision to understand which options were considered and why they were rejected.
- The decision process is focused around the decision text (and not forum-like discussions), so that:
- The resulting text is understandable without reading any discussions.
- There is a common understanding after only reading the decision text.
- To avoid any gaps of reading discussions and the decision.

## Implications

- Proposal issues are obsolete.
- The decision process creates at least:
- two chances to comment decisions, and
- two commits in the git history.
- It might be a barrier for newcomers to write a decision.
This is considered to be okay, as topics that need a decision are not the topics for newcomers.

## Related Decisions

## Notes

- Early discussions in issues or in discussions is not prohibited.
- Discussions in issues/discussions are not prohibited.
They don't bring a decision forward, though.
To not waste time, it is recommended to start with the decision process as described here asap.

Written by Markus Raab 10.10.2022
Written by Markus Raab 10.10.2022.
Second discussion round 28.10.2022.
85 changes: 85 additions & 0 deletions doc/decisions/man_pages.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# Man Pages

## Problem

Our man pages are written as Markdown in `doc/help` and then converted to roff and stored in `doc/man`.
These are the only generated files in our version control system.
Having such files is a problematic workaround, which was introduced because `ronn-ng` is not available on most distributions.
The poor availability of the package `ronn-ng` is a problem because distributions usually build packages by exclusively relying on other packages of the distribution.
E.g. `dpkg-buildpackage` must work with only `deb` packages installed (and not any packages via `gem`, as would be needed to get `ronn-ng v0.10.1`).

We have a mechanism to automatically disable (re)building man pages.
But we want to avoid that distributions build packages without man pages, hence we added the generated files.

Storing generated files, however, is problematic, as it requires:

- developers to always update generated files if the sources are changed
- developers not committing irrelevant changes to generated files (e.g. as may occur with different `CMAKE_INSTALL_PREFIX` etc.)
- require extra effort for continuous integration, e.g. [#4542](https://issues.libelektra.org/4542)

TODO: everything below is draft, please don't comment on it.

## Constraints

1. we want beautiful rendered man pages, e.g., OPTIONS section looks like normal man pages, see in Notes¹ below
2. we cannot require rare tools for the build process: the man pages must be present in every package

## Assumptions

## Considered Alternatives

0. staying with `ronn-ng` but add the man pages only in the release tarballs but not to `git`
1. Write a tool that converts our specification, similar to [pythongen](/src/tools/pythongen/template/template.man)
2. Write a tool that parses gopts `--help` output
3. [help2man](https://www.gnu.org/software/help2man/)
4. Doxygen:
- Constraint 1 probably broken
5. Pandoc:
- has a few standard dependencies
- would need rewrite of the current documentation in doc/help:
- To fulfill Constraint 1 [definition lists](https://pandoc.org/MANUAL.html#definition-lists) would be needed
- would need YAML metadata/front matter for every file
(It would be possible, but not advisable, to:
- also pass information as command-line arguments via `--variable` but then we would move meta-information about man pages to the build system
- that we use the current (non-standard) front matter and convert it to Pandoc's frontmatter but this makes the build system more complicated)

## Decision

Not yet done except spelling of man pages, see [#4567](https://issues.libelektra.org/4567).

## Rationale

## Implications

## Related Decisions

- []()
- []()
- []()

## Notes

¹ ronn-ng converts:

```
- `-H`, `--help`:
Show the man page.
- `-V`, `--version`:
Print version info.
- `-p`, `--profile <profile>`:
Use a different kdb profile.
```

to:

```
OPTIONS
-H, --help
Show the man page.
-V, --version
Print version info.
-p, --profile <profile>
Use a different kdb profile.
```
Loading

0 comments on commit fb593f5

Please sign in to comment.