122 lines
5.3 KiB
Markdown
122 lines
5.3 KiB
Markdown
# Pull requests
|
||
|
||
<div class=pagetoc>
|
||
<!-- toc -->
|
||
</div>
|
||
|
||
Most contributed hledger code (and some of the project maintainer's code)
|
||
is submitted and reviewed via Github pull requests.
|
||
Here are some tips for contributing PRs to hledger.
|
||
|
||
## Code review is important
|
||
|
||
We aim to improve and sustain hledger's quality and maintainability over the long term.
|
||
|
||
Many PRs, especially small ones, and even some big ones, can be merged quickly.
|
||
We love merging good PRs quickly.
|
||
|
||
Some bigger or more risky PRs can require substantial review, discussion, changes, or re-submission.
|
||
Sometimes this is a bigger task than the coding.
|
||
Much valuable design, quality control, and knowledge sharing happens at this time.
|
||
Some PRs get rejected, but their discussion and exploration can still be a useful contribution.
|
||
We very much want to avoid wasted work, but it occasionally happens.
|
||
Our process is evolving and imperfect.
|
||
All of this is normal.
|
||
|
||
We hope you'll see it as a golden opportunity to collaborate with experts,
|
||
share and receive knowledge, refine your design/documentation/code,
|
||
and practice real-world development and communication skills.
|
||
Patience and persistence pays.
|
||
|
||
## The pull request
|
||
|
||
A PR should have a clear purpose, documented in its description. Mention any #ISSUENOs addressed.
|
||
|
||
Don't tackle too much at once.
|
||
Smaller/more focussed PRs can be reviewed quicker and accepted (or rejected) quicker.
|
||
|
||
Consider showing a draft of documentation first (more on this below).
|
||
|
||
## The commit(s)
|
||
|
||
Commits should be easy to review.
|
||
Ideally each commit is complete, and has a single clear purpose,
|
||
which should be documented in the summary (and long description, if needed).
|
||
\#ISSUENOs can be mentioned in summary/description too when appropriate.
|
||
|
||
Within the above constraint, fewer, larger commits are preferred.
|
||
|
||
Keep in mind that commit messages are valuable documentation
|
||
for future developers and troubleshooters.
|
||
They are also the starting point for package changelogs and hledger release notes.
|
||
High-quality commit messages makes the release process quicker, and the resulting docs better.
|
||
|
||
User-impacting commits should mention the user-visible changes,
|
||
and be described in user-relevant language.
|
||
Library-user-impacting commits, eg API changes, ideally will also
|
||
be called out, and can described in more technical language.
|
||
Commits affecting hledger internals are less important,
|
||
but you may notice some adhoc conventions if you browse the history.
|
||
In particular, you can optionally prefix the summary with short component codes (cf [Issues](#issues))
|
||
to facilitate history reading and changelog/release note production.
|
||
|
||
Rewrite and force-push your commits freely (rebase -i, push -f) to clean them up.
|
||
Unless we decide to squash the PR into one commit,
|
||
your commits will become part of hledger's history "for all time",
|
||
so think about future developers trying to understand them, git bisect, etc.
|
||
|
||
Rebase your commits against latest master for easiest review. Especially if they start to conflict.
|
||
|
||
We like to use some conventions in commit messages when it makes sense. These aren't mandatory, but appreciated:
|
||
|
||
- prepend a [topic](#topics) prefix, eg `cli: ` or `journal: `, for clarity and to help with changelog production
|
||
- prepend a semicolon (`;`) to indicate commits that need not be mentioned in changelogs/release notes (as in the Emacs project)
|
||
- append a final `[ci skip]` line to indicate commits that need not trigger a CI build, to reduce carbon emissions from Travis.
|
||
|
||
## The docs
|
||
|
||
PRs should include appropriate updates to reference documentation, unless otherwise agreed.
|
||
Typically this means the manual source files (hledger*/hledger*.m4.md).
|
||
It can also involve
|
||
command line option names and descriptions,
|
||
other --help output,
|
||
hledger's commands list,
|
||
hledger-ui's help dialog,
|
||
hledger-web's help dialog,
|
||
etc.
|
||
Sometimes it means the developer docs, at least the ones in the main repo (READMEs).
|
||
|
||
Reviewers can understand your PR more efficiently once proposed doc changes are provided, and may postpone it otherwise.
|
||
We are happy to help with the docs if needed - just ask.
|
||
|
||
Updating rendered manuals (hledger.{1,info,txt,md,html}) is not required, and probably best avoided to reduce conflicts.
|
||
Updating other docs such as tutorials, how-tos, examples, or screenshots is not required,
|
||
though it's welcome (may be in a different repo).
|
||
|
||
## Documentation first
|
||
|
||
hledger follows documentation-driven design.
|
||
It is in fact highly effective, and highly recommended,
|
||
to write the new docs (help text/user manual/haddocks/developer README..) before writing any code.
|
||
You can share a rough draft on IRC, on the mail list, in an issue comment,
|
||
or in a "WIP" PR starting with just the proposed docs commit.
|
||
|
||
This is often the quickest road to getting something merged into hledger.
|
||
hledger's many parts interact in surprisingly complex ways.
|
||
The documentation-driven working style lets us discuss, clarify and reach a good-enough consensus economically,
|
||
after which coding/review/acceptance can go quicker.
|
||
<!--
|
||
changes can impact past and future users,
|
||
ease of contribution,
|
||
long-term maintenance costs,
|
||
product architecture,
|
||
compatibility with the larger plain text accounting ecosystem,
|
||
etc.
|
||
-->
|
||
|
||
## Related ideas
|
||
|
||
[Neil Mitchell’s Blog - The One PR Per Day Rule](https://neilmitchell.blogspot.com/2019/06/the-one-pr-per-day-rule.html)
|
||
|
||
|