Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CONTRIBUTING.md #679

Merged
merged 11 commits into from
May 4, 2022
97 changes: 97 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
Contributing to Nickel
======================

Welcome, and thanks for considering contributing to the Nickel project!

# Contributing

There are many useful ways to help which don't involve modifying the source of
Nickel:

- Improving the documentation, user-facing or internal, technical or high-level
- Growing the Nickel ecosystem, by providing the community with libraries
(e.g. a collection of contracts for new use cases), augmenting the
standard library, or improving the tooling (such as code editor integration).
- Reviewing changes from other contributors.

The rest of this document is concerned with any changes impacting the `nickel`
repository, which may or may not involve changing the source of Nickel.

# Table of content

1. [Preamble](#preamble)
1. [Resources](#resources)
1. [Setup a development environment](#set-up-a-development-environment)
1. [How to submit changes](#how-to-submit-changes)

# Preamble

Before contributing any non trivial change to this repository, please first
check for the existence of related work such as issues or open pull requests. If
there are none, it's better to discuss the change you wish to make via an issue,
by email, or using any other method with the maintainers of this repository
(listed below) before actually submitting something.

# Resources

## Documentation

The following resources are oriented toward Nickel users:

- The [README](./README.md) and the [design rationale](./RATIONALE.md)
- The [blog post serie][blog-series] and the [release blog post][blog-release].
- The [user manual][user-manual].

For Nickel contributors (or aspiring contributors), the following technical
documentation is relevant as well:

- The [crate documentation][doc-crate].
- The [RFCs][rfcs]. There is currently no well established process for RFCs, but as a
rule of thumb, impactful changes to the design or the implementation of the
language are technically discussed and documented in a dedicated RFC document.
- The [technical notes][doc-notes]. Various notes gathering thoughts, proposals,
or issues about an aspect of the language at one point in time that the author
thought important to keep for posterity. They are usually more informal than
RFCs, of smaller scope, and their content may become obsolete more easily.

## People

Nickel is maintained by [Tweag][tweag]. The current lead maintainer is Yann
Hamdaoui (@yannham).

You can find some of us on our [matrix channel][matrix-nickel] (and in
particular the Devs room), or fire an email at `nickel-lang@tweag.io`.

# Set up a development environment

Please refer to [`HACKING.md`](./HACKING.md) to setup a development environment
for Nickel, as well as adding or running tests and benchmarks.

# How to submit changes

**Try to keep pull requests small and focused**. Avoid packing refactoring,
cosmetic changes or anything not directly related to your original goal in the
same pull request. If preliminary steps make sense as standalone changes, don't
hesitate to split your pull request into several ones. There are a couple
aspects to consider when working on Nickel:

1. **Documentation**: If you added new items (functions, modules) to the public API
of a crate, please document those items in-code. If you made an user-facing
change (syntax, stdlib, language features, etc.), please update the existing
documentation (in particular the user-manual) in consequence.
2. **Tests**: be it for bug fixing or adding new features, please try to write
extensive tests as much as possible, to ensure the correctness of your
changes as well as avoiding regressions.
3. **Benchmarks**: if your change is likely to impact the performance in a
non-trivial way, it might be useful to run the benchmark suite on both master
and on your branch and to report those results in the description of the PR.

[blog-series]: https://www.tweag.io/blog/2020-10-22-nickel-open-sourcing/
[blog-release]: https://www.tweag.io/blog/2022-03-11-nickel-first-release/
[user-manual]: https://nickel-lang.org/user-manual/introduction/
[doc-crate]: https://docs.rs/nickel-lang/0.1.0/nickel_lang/
[doc-notes]: notes/
[tweag]: https://www.tweag.io
[rfcs]: ./rfcs/
[matrix-nickel]: https://matrix.to/#/#nickel-lang:matrix.org
[nickel-lang.org]: https://nickel-lang.org
198 changes: 198 additions & 0 deletions HACKING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
The Nickel developer guide
==========================

There are two ways to set up a development environment for Nickel: using
[Nix][nix], or directly via your preferred system package manager. Nix is able
to drop you in a development shell with everything needed (the Rust toolchain,
mostly) to hack on Nickel in one command, without installing anything globally
on your system. _While Nix is also capable of building Nickel by itself, using
cargo (either a system-wide installation or the one given by the Nix development
shell) is the recommended way of building when working on the Nickel repository
itself_. The reason is that incremental compilation for Rust and Nix is not
there yet, and incremental rebuilds using only Nix are going to be painfully
long.

# Content

The Nickel repository consist in 3 crates:

- `nickel-lang` (path: `.`). The main crate containing the interpreter as a library as well as the `nickel` binary.
- `nickel-lang-lsp` (path: `lsp/nls/`). the Nickel Language Server (NLS), an LSP server for Nickel.
- `nickel-lang-utilities`: (path: `utilities/`). An auxiliary crate regrouping
helpers for tests and benchmarks. Not required to build `nickel` itself.

Other noteworthy items:

- The user manual in `doc/manual/`, as a bunch of markdown files.
- A VSCode extension for NLS in `lsp/client-extension/`.

# Setup a development environment

## Using Nix

To set up a development environment using a recent Nix (>= 2.4):

1. Clone the repository: `git clone git@github.com:tweag/nickel.git`
2. At the root of the repository, run `nix develop`. You should now be dropped
in a shell with all the required tool to hack on Nickel (`rust`, `cargo`,
etc.)

## Without Nix

Otherwise, you can install the Rust toolchain separately: follow the
instructions of the [Rust installation guide][install-rust].

# Building

You can build all crates at once:

```shell
$ cargo build --all
$ ./target/debug/nickel --version
nickel-lang 0.1.0
$ ./target/debug/nls --version
nickel-lang-lsp 0.1.0
```

## Nickel

To only build the main crate `nickel-lang`, run:

```shell
$ cargo build
$ ./target/debug/nickel --version
nickel-lang 0.1.0
```

## NLS (nickel-lang-lsp)

To build NLS separately, the LSP server for Nickel, build the `nickel-lang-lsp` crate:

```shell
$ cargo build -p nickel-lang-lsp
$ ./target/debug/nls --version
nickel-lang-lsp 0.1.0
```

(Alternatively, you can run `cargo build` directly inside `lsp/nls/`).

## WebAssembly REPL

There is a WebAssembly (WASM) version of the REPL, which is used for the online
playground on [nickel-lang.org][nickel-lang.org]. Using Cargo directly to build
the WASM REPL is possible but is not a trivial procedure. The Cargo way requires
installing new tools and patching `Cargo.toml`.

On the other hand, Nix can do the whole build in one simple command, but
incremental compilation is not as good as with direct usage of `cargo`.

Both methods are described below.

### Using Nix

At the root of the repository:

```shell
$ nix build .#buildWasm
$ ls result/nickel-repl
LICENSE package.json nickel_lang_bg.js nickel_lang_bg.wasm [..]
```

### Using Cargo

1. [Install `wasm-pack`][install-wasm-pack]
2. Add the following line to `Cargo.toml` under the `[lib]` heading:

```diff
[lib]
+crate-type = ["cdylib", "rlib"]
```

3. Remove the following line from `Cargo.toml` (dependency on
`nickel-lang-utilities`):

```diff
-nickel-lang-utilities = {path = "utilities", version = "0.1.0"}
```

4. Run `wasm-pack`:

```shell
wasm-pack build -- --no-default-features --features repl-wasm
```

A `pkg` directory, containing the corresponding NPM package, should now be
available.
5. (Optional) the generated NPM package is named `nickel`, but this name is not
very descriptive, and is already in use in the NPM registry. You can patch
the name of the NPM package using `jq` to be `nickel-repl` instead:

```shell
$ jq '.name = "nickel-repl"' pkg/package.json > package.json.patched \
&& rm -f pkg/package.json \
&& mv package.json.patched pkg/package.json
```

If you don't have `jq`, you can do it by hand: replace the `name` attribute
in `pkg/package.json` by `"nickel-repl"`.

The `pkg` directory now contains a `nickel-repl` NPM package that can be used
from JavaScript.

# Testing

Tests are run via `cargo test`. They are two types of tests:

- Unit tests, located directly in the corresponding module.
- Integration tests, located in the dedicated crate `tests`.

You can take inspiration from the existing tests to add your own. By convention,
tests expected to pass are written in a standalone Nickel file in `tests/pass/`.
Each `.ncl` file defines a list of expressions that must individually evaluate
to the boolean `true`. The whole file is an expression that returns true if and
only if every tests pass, or fail with a contract failure to help locating the
failing test (instead of returning just `false`).

If a test expected to pass is failing, run it directly using nickel with `nickel
-f tests/pass/test_that_doesnt_pass.ncl` to get better error messages than
`cargo test`.

Tests expected to fail are often embedded directly into rust source code,
because you usually want to additionally check that the error is the one you
expect. For example ([`tests/records_fail.rs`](./tests/records_fail.rs)):

```rust
#[test]
fn non_mergeable() {
assert_matches!(
eval("({a=1} & {a=2}).a"),
Err(Error::EvalError(EvalError::MergeIncompatibleArgs(..)))
);
assert_matches!(
eval("({a | default = false} & {a | default = true}).a"),
Err(Error::EvalError(EvalError::MergeIncompatibleArgs(..)))
);
}
```

# Benchmarking

If your change is likely to impact performance, it is recommended to run the
benchmark suite on master and on your branch to assess any performance changes.
Please report your findings in the description of the PR.

The benchmark suite is located in the `benches/` directory. To run it:

```shell
$ cargo bench
```

Note that a full run takes some time, up to a dozen of minutes. You can run
specific benchmarks instead of the full suite. Please refer to the documentation
of [`cargo bench`][doc-cargo-bench].

[nix]: https://nixos.org/
[install-rust]: https://www.rust-lang.org/tools/install
[install-wasm-pack]: https://rustwasm.github.io/wasm-pack/installer/
[doc-cargo-bench]: https://doc.rust-lang.org/cargo/commands/cargo-bench.html
[nickel-lang.org]: https://nickel-lang.org