From 0833c9018bc0de7ff5916743d851d46a1f3f2eb1 Mon Sep 17 00:00:00 2001 From: Josh McKinney Date: Sat, 8 Jul 2023 12:02:22 -0700 Subject: [PATCH] docs: improve CONTRIBUTING.md (#277) --- CONTRIBUTING.md | 168 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 124 insertions(+), 44 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cf176eb5..c49a8376 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,69 +1,149 @@ -# Fork Status +# Contribution guidelines -## Pull Requests +First off, thank you for considering contributing to Ratatui. -**All** pull requests opened on the original repository have been imported. We'll be going through any open PRs in a timely manner, starting with the **smallest bug fixes and README updates**. If you have an open PR make sure to let us know about it on our [discord](https://discord.gg/pMCEU9hNEj) as it helps to know you are still active. +If your contribution is not straightforward, please first discuss the change you wish to make by +creating a new issue before making the change, or starting a discussion on +[discord](https://discord.gg/pMCEU9hNEj). -## Issues +## Reporting issues -We have been unsuccessful in importing all issues opened on the previous repository. -For that reason, anyone wanting to **work on or discuss** an issue will have to follow the following workflow : +Before reporting an issue on the [issue tracker](https://github.com/tui-rs-revival/ratatui/issues), +please check that it has not already been reported by searching for some related keywords. Please +also check [`tui-rs` issues](https://github.com/fdehau/tui-rs/issues/) and link any related issues +found. -- Recreate the issue -- Start by referencing the **original issue**: ```Referencing issue #[]()``` -- Then, paste the original issues **opening** text +## Pull requests -You can then resume the conversation by replying to this new issue you have created. +All contributions are obviously welcome. Please include as many details as possible in your PR +description to help the reviewer (follow the provided template). Make sure to highlight changes +which may need additional attention or you are uncertain about. Any idea with a large scale impact +on the crate or its users should ideally be discussed in a "Feature Request" issue beforehand. -### Closing Issues +### Keep PRs small, intentional and focused -If you close an issue that you have "imported" to this fork, please make sure that you add the issue to the **CLOSED_ISSUES.md**. This will enable us to keep track of which issues have been closed from the original repo, in case we are able to have the original repository transferred. +Try to do one pull request per change. The time taken to review a PR grows exponential with the size +of the change. Small focused PRs will generally be much more faster to review. PRs that include both +refactoring (or reformatting) with actual changes are more difficult to review as every line of the +change becomes a place where a bug may have been introduced. Consider splitting refactoring / +reformatting changes into a separate PR from those that make a behavioral change, as the tests help +guarantee that the behavior is unchanged. -# Contributing +### Search `tui-rs` for similar work + +The original fork of Ratatui, [`tui-rs`](https://github.com/fdehau/tui-rs/), has a large amount of +history of the project. Please search, read, link, and summarize any relevant +[issues](https://github.com/fdehau/tui-rs/issues/), +[discussions](https://github.com/fdehau/tui-rs/discussions/) and [pull +requests](https://github.com/fdehau/tui-rs/pulls). + +### Use conventional commits + +We use [conventional commits](https://www.conventionalcommits.org/en/v1.0.0/) and check for them as +a lint build step. To help adhere to the format, we recommend to install +[Commitizen](https://commitizen-tools.github.io/commitizen/). By using this tool you automatically +follow the configuration defined in [.cz.toml](.cz.toml). Your commit messages should have enough +information to help someone reading the [CHANGELOG](./CHANGELOG.md) understand what is new just from +the title. The summary helps expand on that to provide information that helps provide more context, +describes the nature of the problem that the commit is solving and any unintuitive effects of the +change. It's rare that code changes can easily communicate intent, so make sure this is clearly +documented. + +### Clean up your commits + +The final version of your PR that will be committed to the repository should be rebased and tested +against main. Every commit will end up as a line in the changelog, so please squash commits that are +only formatting or incremental fixes to things brought up as part of the PR review. Aim for a single +commit (unless there is a strong reason to stack the commits). See [Git Best Practices - On Sausage +Making](https://sethrobertson.github.io/GitBestPractices/#sausage) for more on this. + +### Run CI tests before pushing a PR + +We're using [cargo-husky](https://github.com/rhysd/cargo-husky) to automatically run git hooks, +which will run `cargo make ci` before each push. To initialize the hook run `cargo test`. If +`cargo-make` is not installed, it will provide instructions to install it for you. This will ensure +that your code is formatted, compiles and passes all tests before you push. If you need to skip this +check, you can use `git push --no-verify`. + +### Sign your commits + +We use commit signature verification, which will block commits from being merged via the UI unless +they are signed. To set up your machine to sign commits, see [managing commit signature +verification](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification) +in GitHub docs. ## Implementation Guidelines +### Setup + +Clone the repo and build it using [cargo-make](https://sagiegurari.github.io/cargo-make/) + +Ratatui is an ordinary Rust project where common tasks are managed with +[cargo-make](https://github.com/sagiegurari/cargo-make/). It wraps common `cargo` commands with sane +defaults depending on your platform of choice. Building the project should be as easy as running +`cargo make build`. + +```shell +git clone https://github.com/tui-rs-revival/ratatui.git +cd ratatui +cargo make build +``` + +### Tests + +The [test coverage](https://app.codecov.io/gh/tui-rs-revival/ratatui) of the crate is reasonably +good, but this can always be improved. Focus on keeping the tests simple and obvious and write unit +tests for all new or modified code. Beside the usual doc and unit tests, one of the most valuable +test you can write for Ratatui is a test against the `TestBackend`. It allows you to assert the +content of the output buffer that would have been flushed to the terminal after a given draw call. +See `widgets_block_renders` in [tests/widgets_block.rs](./tests/widget_block.rs) for an example. + +When writing tests, generally prefer to write unit tests and doc tests directly in the code file +being tested rather than integration tests in the `tests/` folder. + +If an area that you're making a change in is not tested, write tests to characterize the existing +behavior before changing it. This helps ensure that we don't introduce bugs to existing software +using Ratatui (and helps make it easy to migrate apps still using `tui-rs`). + ### Use of unsafe for optimization purposes -**Do not** use unsafe to achieve better performances. This is subject to change, [see.](https://github.com/tui-rs-revival/tui-rs-revival/discussions/66) -The only exception to this rule is if it's to fix **reproducible slowness.** - -## Building - -[cargo-make]: https://github.com/sagiegurari/cargo-make "cargo-make" - -`ratatui` is an ordinary Rust project where common tasks are managed with [cargo-make]. -It wraps common `cargo` commands with sane defaults depending on your platform of choice. -Building the project should be as easy as running `cargo make build`. - -## :hammer_and_wrench: Pull requests - -All contributions are obviously welcome. -Please include as many details as possible in your PR description to help the reviewer (follow the provided template). -Make sure to highlight changes which may need additional attention or you are uncertain about. -Any idea with a large scale impact on the crate or its users should ideally be discussed in a "Feature Request" issue beforehand. - -## Committing - -To avoid any issues that may arrise with the CI/CD by not following the [conventional commit](https://www.conventionalcommits.org/en/v1.0.0/) syntax, we recommend to install [Commitizen](https://commitizen-tools.github.io/commitizen/).\ -By using this tool you automatically follow the configuration defined in [.cz.toml](.cz.toml). - -Additionally, we're using [cargo-husky](https://github.com/rhysd/cargo-husky) to automatically load pre-push hook, which will run `cargo make ci` before each push. It will load the hook automatically when you run `cargo test`. If `cargo-make` is not installed, it will install it for you.\ -This will ensure that your code is formatted, compiles and passes all tests before you push. If you want to skip this check, you can use `git push --no-verify`. +We don't currently use any unsafe code in Ratatui, and would like to keep it that way. However there +may be specific cases that this becomes necessary in order to avoid slowness. Please see [this +discussion](https://github.com/tui-rs-revival/ratatui/discussions/66) for more about the decision. ## Continuous Integration We use Github Actions for the CI where we perform the following checks: + - The code should compile on `stable` and the Minimum Supported Rust Version (MSRV). - The tests (docs, lib, tests and examples) should pass. - The code should conform to the default format enforced by `rustfmt`. - The code should not contain common style issues `clippy`. -You can also check most of those things yourself locally using `cargo make ci` which will offer you a shorter feedback loop. +You can also check most of those things yourself locally using `cargo make ci` which will offer you +a shorter feedback loop than pushing to github. -## Tests +## Relationship with `tui-rs` -The test coverage of the crate is far from being ideal but we already have a fair amount of tests in place. -Beside the usual doc and unit tests, one of the most valuable test you can write for `ratatui` is a test against the `TestBackend`. -It allows you to assert the content of the output buffer that would have been flushed to the terminal after a given draw call. -See `widgets_block_renders` in [tests/widgets_block.rs](./tests/widget_block.rs) for an example. +This project was forked from [`tui-rs`](https://github.com/fdehau/tui-rs/) in February 2023, with the +[blessing of the original author](https://github.com/fdehau/tui-rs/issues/654), Florian Dehau +([@fdehau](https://github.com/fdehau)). + +The original repository contains all the issues, PRs and discussion that were raised originally, and +it is useful to refer to when contributing code, documentation, or issues with Ratatui. + +We imported all the PRs from the original repository and implemented many of the smaller ones and +made notes on the leftovers. These are marked as draft PRs and labelled as [imported from +tui](https://github.com/tui-rs-revival/ratatui/pulls?q=is%3Apr+is%3Aopen+label%3A%22imported+from+tui%22). +We have documented the current state of those PRs, and anyone is welcome to pick them up and +continue the work on them. + +We have not imported all issues opened on the previous repository. For that reason, anyone wanting +to **work on or discuss** an issue will have to follow the following workflow: + +- Recreate the issue +- Start by referencing the **original issue**: ```Referencing issue #[]()``` +- Then, paste the original issues **opening** text + +You can then resume the conversation by replying to this new issue you have created.