232 lines
9.8 KiB
Markdown
232 lines
9.8 KiB
Markdown
|
# Contribution Guidelines
|
|||
|
|
|||
|
This document contains guidelines for contributing code to aerc. It has to be
|
|||
|
followed in order for your patch to be approved and applied.
|
|||
|
|
|||
|
## Contribution Channels
|
|||
|
|
|||
|
Anyone can contribute to aerc. First you need to clone the repository and build
|
|||
|
the project:
|
|||
|
|
|||
|
$ git clone https://git.sr.ht/~rjarry/aerc
|
|||
|
$ cd aerc
|
|||
|
$ make
|
|||
|
|
|||
|
Patch the code. Write some tests. Ensure that your code is properly formatted
|
|||
|
with gofumpt. Ensure that everything builds and works as expected. Ensure that
|
|||
|
you did not break anything.
|
|||
|
|
|||
|
- If applicable, update unit tests.
|
|||
|
- If adding a new feature, please consider adding new tests.
|
|||
|
- Do not forget to update the docs.
|
|||
|
- If your commit brings visible changes for end-users, add an entry in the
|
|||
|
*Unreleased* section of the
|
|||
|
[CHANGELOG.md](https://git.sr.ht/~rjarry/aerc/tree/master/item/CHANGELOG.md)
|
|||
|
file.
|
|||
|
- run the linter using `make lint` if notmuch is not available on your system
|
|||
|
you may have to edit `.golangci.toml` and disable the notmuch tag. [Otherwise
|
|||
|
you could get hard to trace false
|
|||
|
positives](https://github.com/golangci/golangci-lint/issues/3061)
|
|||
|
|
|||
|
Once you are happy with your work, you can create a commit (or several
|
|||
|
commits). Follow these general rules:
|
|||
|
|
|||
|
- Limit the first line (title) of the commit message to 60 characters.
|
|||
|
- Use a short prefix for the commit title for readability with `git log --oneline`.
|
|||
|
- Use the body of the commit message to actually explain what your patch does
|
|||
|
and why it is useful.
|
|||
|
- Address only one issue/topic per commit.
|
|||
|
- If you are fixing a ticket, use appropriate
|
|||
|
[commit trailers](https://man.sr.ht/git.sr.ht/#referencing-tickets-in-git-commit-messages).
|
|||
|
- If you are fixing a regression introduced by another commit, add a `Fixes:`
|
|||
|
trailer with the commit id and its title.
|
|||
|
|
|||
|
There is a great reference for commit messages in the
|
|||
|
[Linux kernel documentation](https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes).
|
|||
|
|
|||
|
IMPORTANT: you must sign-off your work using `git commit --signoff`. Follow the
|
|||
|
[Linux kernel developer's certificate of origin][linux-signoff] for more
|
|||
|
details. All contributions are made under the MIT license. If you do not want
|
|||
|
to disclose your real name, you may sign-off using a pseudonym. Here is an
|
|||
|
example:
|
|||
|
|
|||
|
Signed-off-by: Robin Jarry <robin@jarry.cc>
|
|||
|
|
|||
|
Before sending the patch, you should configure your local clone with sane
|
|||
|
defaults:
|
|||
|
|
|||
|
$ git config format.subjectPrefix "PATCH aerc"
|
|||
|
$ git config sendemail.to "~rjarry/aerc-devel@lists.sr.ht"
|
|||
|
|
|||
|
And send the patch to the mailing list ([step by step
|
|||
|
instructions][git-send-email-tutorial]):
|
|||
|
|
|||
|
$ git send-email --annotate -1
|
|||
|
|
|||
|
Before your patch can be applied, it needs to be reviewed and approved by
|
|||
|
others. They will indicate their approval by replying to your patch with
|
|||
|
a [Tested-by, Reviewed-by or Acked-by][linux-review] (see also: [the git
|
|||
|
wiki][git-trailers]) trailer. For example:
|
|||
|
|
|||
|
Acked-by: Robin Jarry <robin@jarry.cc>
|
|||
|
|
|||
|
There is no "chain of command" in aerc. Anyone that feels comfortable enough to
|
|||
|
"ack" or "review" a patch should express their opinion freely with an official
|
|||
|
Acked-by or Reviewed-by trailer. If you only tested that a patch works as
|
|||
|
expected but did not conduct a proper code review, you can indicate it with
|
|||
|
a Tested-by trailer.
|
|||
|
|
|||
|
You can follow the review process via email and on the
|
|||
|
[web ui](https://lists.sr.ht/~rjarry/aerc-devel/patches).
|
|||
|
|
|||
|
Wait for feedback. Address comments and amend changes to your original commit.
|
|||
|
Then you should send a v2 (and maybe a v3, v4, etc.):
|
|||
|
|
|||
|
$ git send-email --annotate -v2 -1
|
|||
|
|
|||
|
Be polite, patient and address *all* of the reviewers' remarks. If you disagree
|
|||
|
with something, feel free to discuss it.
|
|||
|
|
|||
|
Once your patch has been reviewed and approved (and if the maintainer is OK
|
|||
|
with it), it will be applied and pushed.
|
|||
|
|
|||
|
## Code Style
|
|||
|
|
|||
|
Please refer only to the quoted sections when guidelines are sourced from
|
|||
|
outside documents as some rules of the source material may conflict with other
|
|||
|
rules set out in this document.
|
|||
|
|
|||
|
### Indentation
|
|||
|
|
|||
|
Indentation rules follow the Linux kernel coding style:
|
|||
|
|
|||
|
> Tabs are 8 characters, and thus indentations are also 8 characters. […]
|
|||
|
>
|
|||
|
> Rationale: The whole idea behind indentation is to clearly define where
|
|||
|
> a block of control starts and ends. Especially when you’ve been looking at
|
|||
|
> your screen for 20 straight hours, you’ll find it a lot easier to see how the
|
|||
|
> indentation works if you have large indentations.
|
|||
|
> — [Linux kernel coding style][linux-coding-style]
|
|||
|
|
|||
|
### Breaking long lines and strings
|
|||
|
|
|||
|
Wrapping rules follow the Linux kernel coding style:
|
|||
|
|
|||
|
> Coding style is all about readability and maintainability using commonly
|
|||
|
> available tools.
|
|||
|
>
|
|||
|
> The preferred limit on the length of a single line is 80 columns.
|
|||
|
>
|
|||
|
> Statements longer than 80 columns should be broken into sensible chunks,
|
|||
|
> unless exceeding 80 columns significantly increases readability and does not
|
|||
|
> hide information.
|
|||
|
> […]
|
|||
|
> These same rules are applied to function headers with a long argument list.
|
|||
|
>
|
|||
|
> However, never break user-visible strings such as printk messages because
|
|||
|
> that breaks the ability to grep for them.
|
|||
|
> — [Linux kernel coding style][linux-coding-style]
|
|||
|
|
|||
|
Whether or not wrapping lines is acceptable can be discussed on IRC or the
|
|||
|
mailing list, when in doubt.
|
|||
|
|
|||
|
### Functions
|
|||
|
|
|||
|
Function rules follow the Linux kernel coding style:
|
|||
|
|
|||
|
> Functions should be short and sweet, and do just one thing. They should fit
|
|||
|
> on one or two screenfuls of text (the ISO/ANSI screen size is 80x24, as we
|
|||
|
> all know), and do one thing and do that well.
|
|||
|
>
|
|||
|
> The maximum length of a function is inversely proportional to the complexity
|
|||
|
> and indentation level of that function. So, if you have a conceptually simple
|
|||
|
> function that is just one long (but simple) case-statement, where you have to
|
|||
|
> do lots of small things for a lot of different cases, it’s OK to have
|
|||
|
> a longer function.
|
|||
|
>
|
|||
|
> However, if you have a complex function, and you suspect that
|
|||
|
> a less-than-gifted first-year high-school student might not even understand
|
|||
|
> what the function is all about, you should adhere to the maximum limits all
|
|||
|
> the more closely. Use helper functions with descriptive names (you can ask
|
|||
|
> the compiler to in-line them if you think it’s performance-critical, and it
|
|||
|
> will probably do a better job of it than you would have done).
|
|||
|
>
|
|||
|
> Another measure of the function is the number of local variables. They
|
|||
|
> shouldn’t exceed 5-10, or you’re doing something wrong. Re-think the
|
|||
|
> function, and split it into smaller pieces. A human brain can generally
|
|||
|
> easily keep track of about 7 different things, anything more and it gets
|
|||
|
> confused. You know you’re brilliant, but maybe you’d like to understand what
|
|||
|
> you did 2 weeks from now.
|
|||
|
> — [Linux kernel coding style][linux-coding-style]
|
|||
|
|
|||
|
### Commenting
|
|||
|
|
|||
|
Function rules follow the Linux kernel coding style:
|
|||
|
|
|||
|
> Comments are good, but there is also a danger of over-commenting. NEVER try
|
|||
|
> to explain HOW your code works in a comment: it’s much better to write the
|
|||
|
> code so that the working is obvious, and it’s a waste of time to explain
|
|||
|
> badly written code.
|
|||
|
>
|
|||
|
> Generally, you want your comments to tell WHAT your code does, not HOW. Also,
|
|||
|
> try to avoid putting comments inside a function body: if the function is so
|
|||
|
> complex that you need to separately comment parts of it, you should probably
|
|||
|
> go back to [the previous section regarding functions] for a while. You can
|
|||
|
> make small comments to note or warn about something particularly clever (or
|
|||
|
> ugly), but try to avoid excess. Instead, put the comments at the head of the
|
|||
|
> function, telling people what it does, and possibly WHY it does it.
|
|||
|
>
|
|||
|
> When commenting […] API functions, please use the [GoDoc] format. See the
|
|||
|
> [official documentation][godoc-comments] for details.
|
|||
|
> — [Linux kernel coding style][linux-coding-style]
|
|||
|
|
|||
|
### Editor modelines
|
|||
|
|
|||
|
> Some editors can interpret configuration information embedded in source
|
|||
|
> files, indicated with special markers. For example, emacs interprets lines
|
|||
|
> marked like this:
|
|||
|
>
|
|||
|
> -*- mode: c -*-
|
|||
|
>
|
|||
|
> Or like this:
|
|||
|
>
|
|||
|
> /*
|
|||
|
> Local Variables:
|
|||
|
> compile-command: "gcc -DMAGIC_DEBUG_FLAG foo.c"
|
|||
|
> End:
|
|||
|
> */
|
|||
|
>
|
|||
|
> Vim interprets markers that look like this:
|
|||
|
>
|
|||
|
> /* vim:set sw=8 noet */
|
|||
|
>
|
|||
|
> Do not include any of these in source files. People have their own personal
|
|||
|
> editor configurations, and your source files should not override them. This
|
|||
|
> includes markers for indentation and mode configuration. People may use
|
|||
|
> their own custom mode, or may have some other magic method for making
|
|||
|
> indentation work correctly.
|
|||
|
> — [Linux kernel coding style][linux-coding-style]
|
|||
|
|
|||
|
In the same way, files specific to only your workflow (for example the `.idea`
|
|||
|
or `.vscode` directory) are not desired. If a script might be useful to other
|
|||
|
contributors, it can be sent as a separate patch that adds it to the `contrib`
|
|||
|
directory. Since it is not editor-specific, an
|
|||
|
[`.editorconfig`](https://git.sr.ht/~rjarry/aerc/tree/master/item/.editorconfig)
|
|||
|
is available in the repository.
|
|||
|
|
|||
|
### Go-code
|
|||
|
|
|||
|
The Go-code follows the rules of [gofumpt][gofumpt-repo] which is equivalent to
|
|||
|
gofmt but adds a few additional rules. The code can be automatically formatted
|
|||
|
by running `make fmt`.
|
|||
|
|
|||
|
If gofumpt accepts your code it's most likely properly formatted.
|
|||
|
|
|||
|
[git-send-email-tutorial]: https://git-send-email.io/
|
|||
|
[git-trailers]: https://git.wiki.kernel.org/index.php/CommitMessageConventions
|
|||
|
[godoc-comments]: https://go.dev/blog/godoc
|
|||
|
[gofumpt-repo]: https://github.com/mvdan/gofumpt
|
|||
|
[linux-coding-style]: https://www.kernel.org/doc/html/v5.19-rc8/process/coding-style.html
|
|||
|
[linux-review]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes
|
|||
|
[linux-signoff]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
|