From 53e8184298643ec7cfc1b7fa1dea82d7f7f01749 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Wed, 5 Apr 2023 20:07:11 +0200 Subject: [PATCH] Contribution guidelines --- .github/workflows/full-ci.yml | 2 +- Contributing.md | 157 ++++++++++++++++++++++++++++------ ReadMe.md | 24 +++--- 3 files changed, 141 insertions(+), 42 deletions(-) diff --git a/.github/workflows/full-ci.yml b/.github/workflows/full-ci.yml index 3b57638a0..320442742 100644 --- a/.github/workflows/full-ci.yml +++ b/.github/workflows/full-ci.yml @@ -3,7 +3,7 @@ # file, You can obtain one at https://mozilla.org/MPL/2.0/. # Full CI workflow -# Run before merging. Rebases on master to make sure CI passes for latest integration, not only for the PR at the time of creation. +# Runs before merging. Rebases on master to make sure CI passes for latest integration, not only for the PR at the time of creation. name: Full CI diff --git a/Contributing.md b/Contributing.md index c7b97dc41..d0143e1a0 100644 --- a/Contributing.md +++ b/Contributing.md @@ -1,74 +1,175 @@ -# Contributing to `gdext` +# Contributing to gdext -At this stage, we appreciate if users experiment with the library, use it in small projects and report issues and bugs they encounter. +We appreciate if users experiment with the library, use it in small projects and report issues they encounter. +If you intend to contribute code, please read the section _Pull request guidelines_ below, so you spend less time on administrative tasks. -If you plan to make bigger contributions, make sure to discuss them in a [GitHub issue] first. Since the library is evolving quickly, this avoids that multiple people work on the same thing or implement features in a way that doesn't work with other parts. Also don't hesitate to talk to the developers in the `#contrib-gdext` channel on [Discord]! +The rest of the document goes into tools and infrastructure available for development. -## Check script -The script `check.sh` in the project root can be used to mimic a CI run locally. It's useful to run this before you commit, push or create a pull request: +## Pull request guidelines -``` -$ check.sh +### Larger changes need design + +If you plan to make bigger contributions, make sure to discuss them in a [GitHub issue] before opening a pull request (PR). +Since the library is evolving quickly, this avoids that multiple people work on the same thing, or that features don't integrate well, +causing a lot of rework. Also don't hesitate to talk to the developers in the `#contrib-gdext` channel on [Discord]! + + +### One commit per logical change + +This makes it easier to review changes, later reconstruct what happened when and -- in case of regressions -- revert individual commits. +The exception are tiny changes of a few lines that don't bear semantic significance (typos, style, etc.). +Larger code style changes should be split though. + +If your pull request changes a single thing, please squash the commits into one. Avoid commits like "integrate review feedback" or "fix rustfmt". +Instead, use `git commit --amend` or `git rebase -i` and force-push follow-up commits to your branch (`git push --force-with-lease`). +Since we use the _bors_ bot to merge PRs, we can unfortunately not squash commits upon merge. + + +### Draft PRs + +In case you plan to work for a longer time on a feature/bugfix, consider opening a PR as a draft. +This signals that reviews are appreciated, but that the code is not yet ready for merge. +Non-draft PRs that pass CI are assumed to be mergeable (and maintainers may do so). +
+ +# Dev tools + +## Local development + +The script `check.sh` in the project root can be used to mimic a minimal version of CI locally. +It's useful to run this before you commit, push or create a pull request: + +```bash +$ ./check.sh ``` -At the time of writing, this will run formatting, clippy, unit tests and integration tests. More checks may be added in the future. Run `./check.sh --help` to see all available options. +At the time of writing, this will run formatting, clippy, unit tests and integration tests. More checks may be added in the future. +Run `./check.sh --help` to see all available options. If you like, you can set this as a pre-commit hook in your local clone of the repository: -``` +```bash $ ln -sf check.sh .git/hooks/pre-commit ``` -## Unit tests -Because most of `gdext` interacts with the Godot engine, which is not available from the test executable, unit tests (using `cargo test` and the `#[test]` attribute) are pretty limited in scope. +### Unit tests -Because additional flags might be needed, the preferred way to run unit tests is through the `check.sh` script: +Because most of gdext interacts with the Godot engine, which is not available from the test executable, unit tests +(using `cargo test` and the `#[test]` attribute) are pretty limited in scope. They are primarily used for Rust-only logic. -``` +As additional flags might be needed, the preferred way to run unit tests is through the `check.sh` script: + +```bash $ ./check.sh test ``` -## Integration tests - -The `itest/` directory contains a suite of integration tests that actually exercise `gdext` from within Godot. -The `itest/rust` directory is a Rust `cdylib` library project that can be loaded as a GDExtension in Godot, with an entry point for running integration tests. The `itest/godot` directory contains the Godot project that loads this library and invokes the test suite. +### Integration tests + +The `itest` directory contains a suite of integration tests. It is split into two directories: +`rust`, containing the Rust code for the GDExtension library, and `godot` with the Godot project and GDScript tests. + +Similar to `#[test]`, the function annotated by `#[itest]` contains one integration test. There are multiple syntax variations: + +```rust +// Use a Godot API and verify the results using assertions. +#[itest] +fn variant_nil() { + let variant = Variant::nil(); + assert!(variant.is_nil()); +} + +// TestContext parameter gives access to a node in the scene tree. +#[itest] +fn do_sth_with_the_tree(ctx: &TestContext) { + let tree: Gd = ctx.scene_tree.share(); + + // If you don't need the scene, you can also construct free-standing nodes: + let node: Gd = Node3D::new_alloc(); + // ... + node.free(); // don't forget to free everything created by new_alloc(). +} + +// Skip a test that's not yet ready. +#[itest(skip)] +fn not_executed() { + // ... +} + +// Focus on a one or a few tests. +// As soon as there is at least one #[itest(focus)], only focused tests are run. +#[itest(focus)] +fn i_need_to_debug_this() { + // ... +} +``` You can run the integration tests like this: -``` +```bash $ ./check.sh itest ``` Just like when compiling the crate, the `GODOT4_BIN` environment variable can be used to supply the path and filename of your Godot executable. +Otherwise, a binary named `godot4` in your PATH is used. + -## Formatting +### Formatting `rustfmt` is used to format code. `check.sh` only warns about formatting issues, but does not fix them. To do that, run: -``` +```bash $ cargo fmt ``` -## Clippy + +### Clippy `clippy` is used for additional lint warnings not implemented in `rustc`. This, too, is best run through `check.sh`: +```bash +$ ./check.sh clippy ``` -$ check.sh clippy + +## Continuous Integration + +If you want to have the full CI experience, you can experiment as much as you like on your own gdext fork, before submitting a pull request. + +For this, navigate to the file `.github/workflows/full-ci.yml` and change the following lines: + +```yml +on: + push: + branches: + - staging + - trying ``` -## Real +to: + +```yml +on: + push: +``` + +This runs the entire CI pipeline to run on every push. You can then see the results in the _Actions_ tab in your repository. + +Don't forget to undo this before opening a PR! You may want to keep it in a separate commit named "UNDO" or similar. -Certain types in Godot use either a single or double-precision float internally, such as `Vector2`. When using these types we -use the `real` type instead of choosing either `f32` or `f64`. Thus our code is portable between Godot binaries compiled with -`precision=single` or `precision=double`. + +## Build configurations + +### `real` type + +Certain types in Godot use either a single or double-precision float internally, such as `Vector2`. +When working with these types, we use the `real` type instead of choosing either `f32` or `f64`. +As a result, our code is portable between Godot binaries compiled with `precision=single` and `precision=double`. To run the testing suite with `double-precision` enabled you may add `--double` to a `check.sh` invocation: -``` -$ check.sh --double +```bash +$ ./check.sh --double ``` [GitHub issue]: https://github.com/godot-rust/gdext/issues diff --git a/ReadMe.md b/ReadMe.md index caff7046d..733324ba8 100644 --- a/ReadMe.md +++ b/ReadMe.md @@ -28,33 +28,30 @@ At this point, there is **no** support for Android, iOS or WASM. Contributions a ## Getting started -### Toolchain - -You need to have LLVM installed to use `bindgen`, see [the book](https://godot-rust.github.io/book/getting-started/setup.html#llvm) for instructions. +An elaborate tutorial is available [in the book] (still under construction), here is the short version. To find a version of Godot 4, the library expects either an executable of name `godot4` in the PATH, or an environment variable `GODOT4_BIN` containing the path to the executable (including filename). +We currently only have a GitHub version, crates.io releases are planned once more of the foundation is ready. -### Project setup - -We currently only have a GitHub version, crates.io releases are planned once more of the foundation is ready. In your Cargo.toml, add: ```toml -[dependencies] -godot = { git = "https://github.com/godot-rust/gdext", branch = "master" } - [lib] crate-type = ["cdylib"] + +[dependencies] +godot = { git = "https://github.com/godot-rust/gdext", branch = "master" } ``` To get the latest changes, you can regularly run a `cargo update` (possibly breaking). Keep your `Cargo.lock` file under version control, so that it's easy to revert updates. To register the GDExtension library with Godot, you need to create two files relative to your Godot project folder: -1. First, add `res://MyExt.gdextension`, which is the equivalent of `.gdnlib` for GDNative. - +1. First, add `res://MyExt.gdextension`, which is the equivalent of `.gdnlib` for GDNative. + The `[configuration]` section should be copied as-is. The `[libraries]` section should be updated to match the paths of your dynamic Rust libraries. + `{my_ext}` can be replaced with the name of your crate. ```ini [configuration] entry_symbol = "gdext_rust_init" @@ -82,7 +79,7 @@ To register the GDExtension library with Godot, you need to create two files rel We highly recommend to have a look at a working example in the `examples/dodge-the-creeps` directory. This integrates a small game with Godot and has all the necessary steps set up. -API documentation can be generated locally using `cargo doc -p godot --no-deps --open`. +API documentation can be generated locally using `./check.sh doc` (use `dok` instead of `doc` to open the page in the browser). If you need help, join our [Discord] server and ask in the `#help-gdext` channel! @@ -92,7 +89,7 @@ If you need help, join our [Discord] server and ask in the `#help-gdext` channel We use the [Mozilla Public License 2.0][mpl]. MPL tries to find a balance between permissive (MIT, Apache, Zlib) and copyleft licenses (GPL, LGPL). The license provides a lot of freedom: you can use the library commercially and keep your own code closed-source, -i.e. game development is not restricted. The main condition is that if you change godot-rust _itself_, you need to make +i.e. game development is not restricted. The main condition is that if you change godot-rust _itself_, you need to make those changes available (and only those, no surrounding code). @@ -106,3 +103,4 @@ Contributions are very welcome! If you want to help out, see [`Contributing.md`] [Discord]: https://discord.gg/aKUCJ8rJsc [Mastodon]: https://mastodon.gamedev.place/@GodotRust [Twitter]: https://twitter.com/GodotRust +[in the book]: https://godot-rust.github.io/book/gdext/intro \ No newline at end of file