Skip to content

Commit 7fd1f55

Browse files
committed
docs: add CONTRIBUTING.md file
`CONTRIBUTING.md` is a important file that guides new developers through the standards built by krux team through years. This isn't a monad and could be changed at time to time.
1 parent e2a8bf7 commit 7fd1f55

1 file changed

Lines changed: 339 additions & 0 deletions

File tree

CONTRIBUTING.md

Lines changed: 339 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,339 @@
1+
# Contributing to `Krux` firmware
2+
3+
The development of `Krux` firmware is FOSS and community-effort-based and
4+
welcomes contributions from anyone. We are excited you are interested in
5+
helping us bring sovereign and private self-custody to everyone.
6+
7+
We welcome contributions in many forms, including bug reports, feature
8+
requests, code contributions, and documentation improvements, from any
9+
contributors with any level of experience or expertise.
10+
11+
Besides that, we ask that you respect others and follow the process
12+
outlined in this document.
13+
14+
---
15+
16+
## Read the docs carefully
17+
18+
We have a deep and a concise
19+
[documentation for users](https://selfcustody.github.io/krux). Besides
20+
that, not all of them read the documentation properly.
21+
22+
As a developer, don't do that. Read carefully the current documentation
23+
and, if you think it isn't sufficient for the utmost standards, please
24+
[make a PR](https://github.com/selfcustody/krux/pulls/new); it's a very
25+
`good first issue` and a welcomed contribution.
26+
27+
---
28+
29+
## Non `Krux` firmware contributions
30+
31+
The `Krux` growth in the Bitcoin ecosystem is due to developments beyond firmware.
32+
They're built from real, material, and daily use cases from our beloved
33+
community of kruxers. While we love that, we cannot be responsible for any
34+
development made by our users.
35+
36+
Our developers and contributors are listed in both [krux contributors list](https://github.com/selfcustody/krux/contributors)
37+
and [krux-installer contributors list](https://github.com/selfcustody/krux-installer/contributors)
38+
.
39+
40+
---
41+
42+
## Communications Channels
43+
44+
The primary communication channel is the [GitHub repository](https://github.com/selfcustody/krux)
45+
.
46+
47+
---
48+
49+
## Contribution Workflow
50+
51+
The contribution workflow is designed to facilitate cooperation and ensure a
52+
high level of quality in the project and not to enforce any type of dogmatic
53+
procedure on creative level. The process is as follows:
54+
55+
To contribute a patch, the workflow is as follows:
56+
57+
1. Fork Repository
58+
2. Create topic branch
59+
3. Commit patches
60+
61+
### Fork Repository
62+
63+
[Fork the repository](https://github.com/selfcustody/krux):
64+
65+
```bash
66+
# or the recommended method, via ssh:
67+
git clone git@github.com:selfcustody/krux.git
68+
```
69+
70+
Then follow the initial procedures described on [README.md](./README.md).
71+
72+
### Create a topic branch
73+
74+
You always start with a `main` branch initialized. While the
75+
community of developers standardized this as the **stable** development branch
76+
(some projects could use the `master` branch for historical reasons),
77+
the `Krux` firmware team stated the first one as the **stable** branch and the
78+
`develop` as the branch that all developers should start from and experiment with.
79+
After you have had fun and discovered things, create a properly named branch to
80+
identify when we release:
81+
82+
> Bitcoin related projects, like bdk, pdk and floresta use
83+
`<type>/<name>` named branches.
84+
85+
86+
```bash
87+
main -> develop -> chore/task-stuff
88+
-> ci/job-stuff
89+
-> docs/info-new
90+
...
91+
```
92+
93+
So, in command line you could do this, keeping in mind what you will do in
94+
the PR:
95+
96+
```bash
97+
git checkout -b <type/name>
98+
```
99+
100+
### Named branches
101+
102+
A patch is a set of changes (patches or `diffs`) that you propose to `Krux`
103+
firmware developers to include in `develop` branch as a (set) of commits.
104+
105+
In general commits should be atomic and `diffs` should be easy to read.
106+
For this reason do not mix any formatting fixes or code moves with actual code
107+
changes. Further, each commit, individually, should compile and
108+
pass tests whenever possible, in order to ensure that `git bisect` and other automated tools
109+
function properly.
110+
111+
When adding a new feature, ensure that it is covered by functional tests where
112+
possible. Avoiding this will create uncovered code.
113+
114+
When refactoring, structure your PR to make it easy to review and don't
115+
hesitate to split it into multiple small, focused PRs.
116+
117+
#### Minimum Supported Python Version
118+
119+
The Minimum Supported Python Version is **3.12.0** (enforced by our CI).
120+
Commits should cover both the issue fixed and the solution's rationale.
121+
122+
#### Conventional commits
123+
124+
These [guidelines](https://chris.beams.io/posts/git-commit/) should be kept in
125+
mind. Commit messages follow the
126+
["Conventional Commits 1.0.0"](https://www.conventionalcommits.org/en/v1.0.0/)
127+
to make commit histories easier to read by humans and automated tools.
128+
The types of commits we use are:
129+
130+
- chore: maintenance tasks (mostly lint, format);
131+
- ci: continuous integration (generally `.github/**` files);
132+
- docs: documentation changes (`*.md` files);
133+
- feat: new feature (`src/**`, imperatively -- it's recommended that `test` and
134+
`docs` are accompanied in the PR);
135+
- fix: bug fix (see if it could break changes -- `!`, it's recommended that
136+
`test` and `docs` are accompanied);
137+
- refactor: code change that neither fixes a bug nor adds a feature (could be
138+
a tiny change or a entire code-base refactor -- in krux refactor is the
139+
first);
140+
- style: formatting, missing semi colons, ui colors, icons, etc; no functional
141+
code change;
142+
- test: adding missing tests or correcting existing tests.
143+
144+
#### Signing commits
145+
146+
It is strongly encouraged that you
147+
[GPG sign](https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits)
148+
your commits. *Making a habit of signing your Git commits (...) gives you the
149+
ability to prove that you were the author of a specific code change*. [^1]
150+
151+
---
152+
153+
## Peer review
154+
155+
To make sure our code has the highest quality and is maintainable for
156+
posterity, we have a thorough peer review process, where pull requests need
157+
to be reviewed by at least one maintainer, and must not have any outstanding
158+
comment from regular contributors (unless one of maintainers recognizes the
159+
rationale).
160+
161+
### Conceptual Review
162+
163+
A review can be a conceptual review, where the reviewer leaves a comment:
164+
165+
- Concept `cACK`: "I agree with the general goal of this pull request";
166+
- Approach `nACK`: "I do (not) agree with the approach of this change";
167+
- Untested `utACK`: "I didn't test, but ACK";
168+
- Tested `tACK`: "I tested and ACKed".
169+
170+
A nACK needs to include a rationale why the change is not worthwhile.
171+
nACKs without accompanying reasoning may be disregarded.
172+
173+
### Code Review
174+
175+
After conceptual agreement on the change, code review can be provided.
176+
A review begins with ACK `BRANCH_COMMIT`, where `BRANCH_COMMIT` is the top of
177+
the PR branch, followed by a description of how the reviewer did the review.
178+
The following language is used within pull request comments:
179+
180+
- Tested ACK:
181+
182+
> tACK `babc00fe`. I have tested the code", involving change-specific manual
183+
testing and in case it was not obvious how the manual testing was done. nit:
184+
maybe it could be described in a `docs` commit sharing your intentions.
185+
186+
-- Untested ACK:
187+
188+
> utACK `babc00fe`.
189+
"I have not tested the code, but I have reviewed it and it looks OK, I agree
190+
it can be merged".
191+
192+
A "nit" refers to a trivial, often non-blocking issue.
193+
194+
Project maintainers reserve the right to weigh the opinions of peer reviewers
195+
using common sense judgement and may also weigh based on merit. Reviewers that
196+
have demonstrated a deeper commitment and understanding of the project over
197+
time or who have clear domain expertise may naturally have more weight, as one
198+
would expect in all walks of life.
199+
200+
```markdown
201+
tACK c00febab
202+
203+
I liked the approach! Just some minor nit:
204+
205+
- the line 3 has a typo on comment: `Helllo` should be `Hello`. So
206+
I suggest this change:
207+
208+
<diff>
209+
- # The resulting Helllo
210+
+ # The resulting Hello
211+
result = "Hello"
212+
</diff>
213+
214+
Besides that, is a neat work @user!
215+
```
216+
217+
---
218+
219+
## Coding Conventions
220+
221+
There are a few rules to make sure the code is readable and maintainable.
222+
Most of them are checked by `poetry run poe lint` and `poetry run poe format`,
223+
and are enforced by CI, both for python and markdown code.
224+
225+
### Python
226+
227+
```python
228+
# The MIT License (MIT)
229+
230+
# Copyright (c) 2021-2026 Krux contributors
231+
232+
# ...
233+
class Foo:
234+
"""Some awesome comment"""
235+
236+
def func(self):
237+
"""Some awesome comment"""
238+
pass
239+
```
240+
241+
#### MIT License
242+
243+
All files should have the MIT license with updated years from when we stated
244+
the project (2021) until today (2026 when writing this document).
245+
246+
#### New features
247+
248+
All new features require testing. Tests should be unique and self-describing.
249+
When it comes error handling, we prefer exact and meaningful error handling
250+
to deliver consumers (developers and users) an accurate error that describes
251+
exactly what happened wrong. Instead of:
252+
253+
```python
254+
# A function that concatenate a int and a float into a str
255+
def foo(
256+
bar: int,
257+
baz: float,
258+
) -> str:
259+
str(bar) + str(baz)
260+
```
261+
262+
prefer:
263+
264+
```python
265+
# A function that concatenate a int and a float into a str and why do that
266+
# for a good and reasonable reason that you should discuss with other developers
267+
# keep in mind that we're coding for micropython
268+
def foo(bar, baz):
269+
"""Serialize an int with a float into new str
270+
271+
Args
272+
----
273+
bar(int): an int to be serialized
274+
baz(float): a float to be serialized
275+
276+
Returns
277+
-------
278+
str: the serialized "<int><float>"
279+
"""
280+
if not instanceof(bar, int):
281+
raise ValueError("Expected bar to be an int")
282+
283+
if not instanceof(baz, float):
284+
raise ValueError("Expected baz to be a float")
285+
286+
bar_str = str(bar)
287+
baz_str = str(baz)
288+
289+
# We need to concatenate to be compliant
290+
return bar_str + baz_str
291+
```
292+
293+
### Markdown
294+
295+
Markdown files are linted too. This keeps the `*.md` files standardized and
296+
properly formatted for any code editor. Some of the currently applied rules
297+
enforced by CI:
298+
299+
```markdown
300+
- \# heading should be toplevel heading;
301+
- lines should be compact: 80 chars per line;
302+
- raw links aren't allowed, use the [text](link) format instead;
303+
- items should follow correct indentation.
304+
```
305+
306+
---
307+
308+
## Testing
309+
310+
We expect to have 95% test coverage for each PR. We have a few types of tests
311+
that you can run using `poetry run poe test`. See more with `poetry run poe`.
312+
313+
---
314+
315+
## Release
316+
317+
Once a maintainer and the contributors decide we have a stable enough `develop`
318+
with sufficient features, we will merge the `develop` branch with the `main`
319+
at that point. From this point, all new changes will go in the next release.
320+
After sufficient testing and making sure we don't have bugs left, this branch
321+
will be released by one of the maintainers.
322+
323+
The release will have pre-built binaries available on github's asset page.
324+
They **must** be OpenSSL signed by [odudex](mailto:odudex@proton.me) --
325+
verifiable with [`selfcustody.pem`](./selfcustody.pem), and have a `zip`
326+
accompanied with `zip.sha256.txt` release assets.
327+
328+
If we find bugs on a release, the fix may be backported and a new minor release
329+
may be released. This is done by merging fixes on top of the release branch.
330+
And then performing another release on that branch.
331+
332+
If you found any security issue, please read [`SECURITY.md`](./SECURITY.md),
333+
we're glad you have the choice to grow this community in a cypherpunk way.
334+
335+
If you have any questions related to this process or the codebase in general,
336+
don't hesitate to reach out to us. We are happy to help newcomers in their amazing
337+
journey. Overall, have fun :)
338+
339+
[^1]: https://withblue.ink/2020/05/17/how-and-why-to-sign-git-commits.html

0 commit comments

Comments
 (0)