Merge pull request #229 from quantified-uncertainty/contributing-april-update
Update CONTRIBUTING.md
This commit is contained in:
commit
7ce3712b78
|
@ -52,3 +52,78 @@ See [here](https://github.com/NixOS/nixpkgs/issues/107375)
|
||||||
# Pull request protocol
|
# Pull request protocol
|
||||||
|
|
||||||
Please work against `staging` branch. **Do not** work against `master`. Please do not merge without approval from some subset of Quinn, Sam, and Ozzie; they will be auto-pinged.
|
Please work against `staging` branch. **Do not** work against `master`. Please do not merge without approval from some subset of Quinn, Sam, and Ozzie; they will be auto-pinged.
|
||||||
|
|
||||||
|
# Code Quality
|
||||||
|
- Aim for at least 8/10* quality in ``/packages/squiggle-lang``, and 7/10 quality in ``/packages/components``.
|
||||||
|
- If you submit a PR that is under a 7, for some reason, describe the reasoning for this in the PR.
|
||||||
|
|
||||||
|
* This quality score is subjective.
|
||||||
|
|
||||||
|
# Rescript Style
|
||||||
|
|
||||||
|
**Use `->` instead of `|>`**
|
||||||
|
Note: Our codebase used to use `|>`, so there's a lot of that in the system. We'll gradually change it.
|
||||||
|
|
||||||
|
**Use `x -> y -> z` instead of `let foo = y(x); let bar = z(foo)`**
|
||||||
|
|
||||||
|
**Don't use anonymous functions with over three lines**
|
||||||
|
Bad:
|
||||||
|
```rescript
|
||||||
|
foo
|
||||||
|
-> E.O.fmap(r => {
|
||||||
|
let a = 34;
|
||||||
|
let b = 35;
|
||||||
|
let c = 48;
|
||||||
|
r + a + b + c
|
||||||
|
}
|
||||||
|
```
|
||||||
|
Good:
|
||||||
|
```rescript
|
||||||
|
let addingFn = (r => {
|
||||||
|
let a = 34;
|
||||||
|
let b = 35;
|
||||||
|
let c = 48;
|
||||||
|
r + a + b + c
|
||||||
|
}
|
||||||
|
foo -> addingFn
|
||||||
|
```
|
||||||
|
|
||||||
|
**Write out types for everything, even if there's an interface file**
|
||||||
|
We'll try this for one month (ending May 5, 2022), then revisit.
|
||||||
|
|
||||||
|
**Use the Rescript optional default syntax**
|
||||||
|
Rescript is clever about function inputs. There's custom syntax for default and optional arguments. In the cases where this applies, use it.
|
||||||
|
|
||||||
|
From https://rescript-lang.org/docs/manual/latest/function:
|
||||||
|
```rescript
|
||||||
|
// radius can be omitted
|
||||||
|
let drawCircle = (~color, ~radius=?, ()) => {
|
||||||
|
setColor(color)
|
||||||
|
switch radius {
|
||||||
|
| None => startAt(1, 1)
|
||||||
|
| Some(r_) => startAt(r_, r_)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Use named arguments**
|
||||||
|
If a function is called externally (in a different file), and has either:
|
||||||
|
1. Two arguments of the same type
|
||||||
|
2. Three paramaters or more.
|
||||||
|
|
||||||
|
**Module naming: Use x_y as module names**
|
||||||
|
For example: ``Myname_Myproject_Add.res``. Rescript/Ocaml both require files to have unique names, so long names are needed to keep different parts separate from each other.
|
||||||
|
|
||||||
|
See [this page](https://dev.to/yawaramin/a-modular-ocaml-project-structure-1ikd) for more information. (Though note that they use two underscores, and we do one. We might refactor that later.
|
||||||
|
|
||||||
|
**Module naming: Don't rename modules**
|
||||||
|
We have some of this in the Reducer code, but generally discourage it.
|
||||||
|
|
||||||
|
**Use interface files (.resi) for files with very public interfaces**
|
||||||
|
|
||||||
|
### Recommended Rescript resources
|
||||||
|
- https://dev.to/yawaramin/a-modular-ocaml-project-structure-1ikd
|
||||||
|
- https://github.com/avohq/reasonml-code-style-guide
|
||||||
|
- https://cs.brown.edu/courses/cs017/content/docs/reasonml-style.pdf
|
||||||
|
- https://github.com/ostera/reason-design-patterns/
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue
Block a user