Skip to content

Commit 78af552

Browse files
jmmvjmmv
committed
Add guidelines for contributions
Now that we have one person (wohoo!) interested in contributing to the project, it's time to add some guidelines on how things should look like (and why they look like they do today). While doing this, recommend and configure the Rewrap extension, which is useful to write long paragraphs in Markdown files.
1 parent eea8e0e commit 78af552

3 files changed

Lines changed: 237 additions & 1 deletion

File tree

‎.vscode/extensions.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"bungcip.better-toml",
44
"davidanson.vscode-markdownlint",
55
"matklad.rust-analyzer",
6+
"stkb.rewrap",
67
"streetsidesoftware.code-spell-checker"
78
]
89
}

‎.vscode/settings.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@
2424
"[markdown]": {
2525
"editor.rulers": [80],
2626
"editor.quickSuggestions": false,
27-
"editor.wordWrapColumn": 80
27+
"editor.wordWrapColumn": 80,
28+
"rewrap.doubleSentenceSpacing": true
2829
},
2930

3031
"markdownlint.config": {

‎CONTRIBUTING.md‎

Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
# Contributing guidelines
2+
3+
So you want to contribute to EndBASIC? That's great! But first, please take a
4+
few minutes to read this document in full. Doing so upfront will minimize the
5+
turnaround time required to get your changes incorporated.
6+
7+
## Disclaimers
8+
9+
* EndBASIC is my ([jmmv](https://github.com/jmmv/)'s) personal project and is
10+
developed and maintained in my very limited free time. My energy to work on
11+
this project comes in bursts. Please understand that dealing with
12+
contributions is time-consuming and, because of the above, responses can be
13+
delayed. Also please understand that, right now, some contributions may not
14+
be accepted if they don't align with my vision for the project (which, I
15+
know, should be better documented but I'll be happy to explain when
16+
necessary).
17+
18+
* Even though EndBASIC is a side project, I want to practice and enforce sound
19+
engineering principles. The project is not bound by schedules so there is
20+
no good reason to rush things. That said, the project must ship often! My
21+
personal choice is to always do the right thing at the individual PR level
22+
but evaluate and possibly tolerate known issues at release time.
23+
24+
* As the primary author of the project, I will violate some of the rules
25+
below. In particular, I will usually upload multiple commits in a single PR
26+
and rebase-merge them into the main branch once they pass testing. This is
27+
an "optimization" over the "1 PR = 1 commit" guideline described below
28+
because I have no other code owners to do reviews for me.
29+
30+
## General guidelines
31+
32+
* Before you start working on a large contribution---especially if there is no
33+
matching GitHub issue describing the feature gap---you should get in touch
34+
first. Prefer to file a new issue and keep the discussion there so that
35+
anyone can follow. However, if you are looking for a more informal method
36+
to get started, pick you favorite communication channel from [the Community
37+
section](https://www.endbasic.dev/community.html) of the website.
38+
Coordinating upfront makes it much easier to avoid frustrations later on.
39+
40+
* All changes are subject to thorough code reviews and CI validation before
41+
they are merged.
42+
43+
* I expect quality and attention to detail in every PR. Please watch out for
44+
indentation inconsistencies, typos, grammar mistakes, and the like. While
45+
these do not affect the functionality of your change, problems in that area
46+
make it difficult for me to trust that the rest of your contribution is
47+
correct. See the [Broken windows
48+
theory](https://en.wikipedia.org/wiki/Broken_windows_theory). Because of
49+
this, I will welcome even the most trivial PR to address simple problems!
50+
51+
* I am happy to mentor you through the review process via extensive comments,
52+
but I understand that this can get frustrating at some point. If so, please
53+
let me know. But see next item.
54+
55+
* In general, if your PR is reasonable but only contains a few nits (e.g.
56+
minor style problems or typos), I reserve the right to take your PR and
57+
amend it with those fixes to avoid pointless round trips. *You will
58+
maintain authorship, of course.*
59+
60+
* Good PRs do one thing, and only one thing. Refrain from touching
61+
surrounding lines "just because". If there are nearby details that need
62+
fixing, please send separate PRs.
63+
64+
* Any major changes should be accompanied by an entry to the `NEWS.md` file.
65+
The entry should be added to the section for the `X.Y.Z` pseudo-release at
66+
the top of the file, which is added after every major release to start
67+
accumulating changes. Sentences in this file are written in the past tense.
68+
69+
* Any new functions or commands must be recorded in the `NEWS.md` file, but
70+
also in the `README.md` file of the crate where they belong.
71+
72+
## Git and Pull Request workflows
73+
74+
* The `master` branch is protected and cannot be pushed to directly (not even
75+
by me). All contributions must go through a PR first and pass all testing.
76+
77+
* Assume that "1 PR = 1 commit" in the main branch. This is the preferred
78+
contribution mechanism as it helps ensure commits are focused and small.
79+
80+
* To facilitate code reviews via the GitHub interface, follow these steps:
81+
82+
1. Create a new branch named after the change you intend to do under your
83+
personal branch of the project. If there is a matching issue, prefer
84+
naming your branch `issue-N`.
85+
86+
1. Prepare a PR with a single commit in it under that branch. It is
87+
perfectly fine to squash and force-push changes to *your branch* until
88+
you are ready to start the review process. (In fact, it's perfectly
89+
fine to rewrite history in auxiliary branches until they are merged into
90+
`master`. Don't believe any comments out there that say force-pushing
91+
is never appropriate!)
92+
93+
1. Once the PR is created, prefer to address review comments via
94+
incremental commits with messages like `Address review comments`, as
95+
this makes it possible to review incremental changes to the PR. These
96+
commit messages don't matter because they will be lost once squashed.
97+
(Exception: if the review comments mean that you must change the PR
98+
significantly, squash them all and force-push the branch to get a clean
99+
diff from `HEAD` in GitHub.)
100+
101+
1. Once the PR is approved, it will be squash-merged into the main branch.
102+
The commit message that will appear on the main branch will be based on
103+
the summary and description of the PR---which by default are derived
104+
from *the first commit* of the branch *at PR creation time*. Make sure
105+
to keep these clean following the guidelines below.
106+
107+
* If you feel that you want to submit multiple separate commits at once and
108+
that those should be added to the main branch without squashing them, that's
109+
an option too---but this should be very, very rare. This workflow doesn't
110+
play well with GitHub, makes reviewing subsequent iterations of the changes
111+
very difficult, and makes it very hard for me to fix small details for you.
112+
113+
## Commit messages
114+
115+
* Follow standard Git commit message guidelines. In particular:
116+
117+
* The first line is the summary or subject of the commit and has a maximum
118+
length of 50 characters, does not terminate in a period, and has to
119+
summarize the whole commit. Write this line in the imperative tense,
120+
like `Add foo-bar` or `Fix baz`, instead of `Adding blah`, `Adds bleh`
121+
or `Added bloh`.
122+
123+
* The first line is followed by an empty line and then multiple free-form
124+
paragraphs providing details on the commit. These should be wrapped at
125+
72-75 columns.
126+
127+
* Make sure the verbose explanation of the commit explains the *what* and the
128+
*why* of the change. The *how* is explained by the code change itself so,
129+
in general, you should not have to cover that.
130+
131+
* If you find yourself writing sentences of the form "Also do..." or "This
132+
also fixes...", it probably means that your PR is too broad in scope.
133+
Remember to focus each PR on one semantical change.
134+
135+
* Commits that address an existing issue should end with a single paragraph of
136+
the form `Fixes #N`. More on this below.
137+
138+
* As an example:
139+
140+
```plain
141+
Brief summary (max 50 columns)
142+
143+
This PR does this and that. The reason for this change is this and that
144+
other thing. (Keep paragraph wrapped at 72-75 columns).
145+
146+
Some other paragraph with more details.
147+
148+
Fixes #N.
149+
```
150+
151+
* GitHub is a terrible editor for commit messages and PR descriptions. I
152+
strongly suggest you to compose the message of the first commit in your
153+
branch with Vim *before you create the PR*. Then, when you create the PR,
154+
keep this message unedited in the PR summary and description. (This will
155+
only work if you kept the branch to one commit as described earlier.)
156+
157+
* If you don't want to believe me but believe Linus Torvalds, please see [his
158+
advice on this
159+
topic](https://github.com/torvalds/subsurface-for-dirk/blob/a48494d2fbed58c751e9b7e8fbff88582f9b2d02/README#L88).
160+
161+
## Handling bug tracker issues
162+
163+
* Changes should cross-reference one or more issues in the bug tracker, if
164+
they exist. This is particularly important for known bugs, but also applies
165+
to major feature improvements.
166+
167+
* Unless you have a good reason to do otherwise, name your branch `issue-N`
168+
where `N` is the number of the issue being fixed.
169+
170+
* When fixing an existing issue, add an entry of the form `Issue #X: Fixed
171+
blah.` to the `NEWS.md` file. Do not worry too much about the relative
172+
ordering of the entries: I'll reorder them and possibly reword them too at
173+
the time of release to tell a cohesive story. Similarly, don't forget to
174+
add `Fixes #X.` at the end of the PR description so that, when it's merged,
175+
GitHub links the commit to the issue and closes the issue.
176+
177+
* If your PR partially addresses an issue but does not completely fix it, you
178+
cannot say `Fixes #X.` because that would close the issue. Instead, say
179+
something like `Partially addresses #X.` to establish a link between the
180+
two.
181+
182+
## Coding style
183+
184+
* This project uses `rustfmt` to enforce the style of Rust files so you should
185+
not have to worry about that. Don't fight the auto-formatter even if it
186+
seems to be doing an ugly job.
187+
188+
* The auto-formatter is not perfect though. The only thing you should
189+
manually deal with are comment line lengths, which should be limited to 100
190+
characters.
191+
192+
* Unfortunately, there is no auto-formatter (yet) for all other file types in
193+
the project, including Javascript, HTML, and Markdown. For those, follow
194+
existing style within the file you are touching. I strongly suggest you use
195+
VSCode to work on the project, as the `settings.json` file included in the
196+
source tree will aid you in doing the right thing. Don't forget to install
197+
the recommended extensions.
198+
199+
* Use proper punctuation for all sentences. Always start comments with a
200+
capital letter and terminate them with a period.
201+
202+
* Respect lexicographical sorting wherever possible, unless ordering matters.
203+
204+
* Lines must not be over 100 characters except for in-tree Markdown documents
205+
(like this one) where the limit is 80.
206+
207+
* No trailing whitespace.
208+
209+
* Two spaces after end-of-sentence periods in documents, comments and even
210+
commit messages.
211+
212+
## Testing
213+
214+
* Unit tests (those that live within the same `.rs` file where the changed
215+
code lives) are expected for every code change.
216+
217+
* Integration tests (those under the `tests` subdirectory of a crate) are
218+
expected for a minority of changes. It is hard to describe what the
219+
criteria are in this document. Try to get a sense from what exists today.
220+
221+
* The only exception to writing tests are changes to the web interface because
222+
tests in that space are already lacking. (In other words: I won't ask you
223+
to fix existing deficiencies if you are already modifying a deficient part
224+
of the tree.) However, if you *do* want to increase test coverage for this
225+
part of the codebase so that we can remove this entry, please do so!
226+
227+
* Not writing a test because it's too difficult to do can be reasonable at
228+
times. Note that this should be the exception, not the norm, and should be
229+
justified in the PR description.
230+
231+
* If you add any new built-in command or function, you have to register it in
232+
the `help.in` file. This helps ensure that the built-in multi-line strings
233+
are correctly formatted because the corresponding `help.out` allows visual
234+
inspection of the results.

0 commit comments

Comments
 (0)