Cyclic0007/ratatui
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

chore: update contributing guidelines and add copilot-instructions (#1998)

Browse Source 
See
https://docs.github.com/en/copilot/how-tos/agents/copilot-code-review/using-copilot-code-review

These instructions will be used by copilot when it performs automated PR
reviews, and helps provide guardrails for our standards. Over time we
might grow these to capture any consistent problems that we start seeing
when reviewing.

---------

Co-authored-by: Orhun Parmaksız <orhun@archlinux.org>
This commit is contained in:
Josh McKinney
2025-07-19 16:02:13 -07:00
committed by GitHub
parent cdf189e15a
commit 572749f388
2 changed files with 145 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

81
.github/copilot-instructions.md vendored Normal file
View File

@@ -0,0 +1,81 @@
# GitHub Copilot Code Review Instructions
## General Review Principles
### Pull Request Size and Scope
- **Flag large PRs**: Comment if a PR changes more than 500 lines or touches many unrelated areas
- **Suggest splitting**: Recommend breaking large changes into smaller, focused PRs
- **Question scope creep**: Ask about unrelated changes that seem outside the PR's stated purpose
### Code Quality and Style
- **Check adherence to style guidelines**: Verify changes follow the project's Rust conventions in [CONTRIBUTING.md](https://github.com/ratatui/ratatui/blob/main/CONTRIBUTING.md#code-formatting)
- **Verify xtask compliance**: Ensure `cargo xtask format` and `cargo xtask lint` would pass
- **Look for AI-generated patterns**: Be suspicious of verbose, overly-commented, or non-idiomatic code
### Architectural Considerations
- **Reference ARCHITECTURE.md**: Point to [ARCHITECTURE.md](https://github.com/ratatui/ratatui/blob/main/ARCHITECTURE.md) for changes affecting crate boundaries
- **Question fundamental changes**: Flag modifications to core configuration, linting rules, or build setup without clear justification
- **Verify appropriate crate placement**: Ensure changes are in the correct crate per the modular structure
### Breaking Changes and Deprecation
- **Require deprecation**: Insist on deprecation warnings rather than immediate removal of public APIs
- **Ask for migration path**: Request clear upgrade instructions for breaking changes
- **Suggest feature flags**: Recommend feature flags for experimental or potentially disruptive changes
- **Reference versioning policy**: Point to the requirement of at least one version notice before removal
### Testing and Documentation
- **Verify test coverage**: Ensure new functionality includes appropriate tests
- **Check for test removal**: Question any removal of existing tests without clear justification
- **Require documentation**: Ensure public APIs are documented with examples
- **Validate examples**: Check that code examples are minimal and follow project style
### Specific Areas of Concern
#### Configuration Changes
- **Lint configuration**: Question changes to `.clippy.toml`, `rustfmt.toml`, or CI configuration
- **Cargo.toml modifications**: Scrutinize dependency changes or workspace modifications
- **Build system changes**: Require justification for xtask or build process modifications
#### Large Code Additions
- **Question necessity**: Ask if large code additions could be implemented more simply
- **Check for duplication**: Look for code that duplicates existing functionality
- **Verify integration**: Ensure new code integrates well with existing patterns
#### File Organization
- **Validate module structure**: Ensure new modules follow the project's organization
- **Check import organization**: Verify imports follow the std/external/local grouping pattern
- **Review file placement**: Confirm files are in appropriate locations per ARCHITECTURE.md
## Comment Templates
### For Large PRs
```
This PR seems quite large with changes across multiple areas. Consider splitting it into smaller, focused PRs:
- Core functionality changes
- Documentation updates
- Test additions
- Configuration changes
See our [contribution guidelines](https://github.com/ratatui/ratatui/blob/main/CONTRIBUTING.md#keep-prs-small-intentional-and-focused) for more details.
```
### For Breaking Changes
```
This appears to introduce breaking changes. Please consider:
- Adding deprecation warnings instead of immediate removal
- Providing a clear migration path in the PR description
- Following our [deprecation policy](https://github.com/ratatui/ratatui/blob/main/CONTRIBUTING.md#deprecation-notice)
```
### For Configuration Changes
```
Changes to project configuration (linting, formatting, build) should be discussed first. Please explain:
- Why this change is necessary
- What problem it solves
- Whether it affects contributor workflow
```
### For Style Issues
```
Please run `cargo xtask format` and `cargo xtask lint` to ensure code follows our style guidelines. See [CONTRIBUTING.md](https://github.com/ratatui/ratatui/blob/main/CONTRIBUTING.md#code-formatting) for details.
```

65
CONTRIBUTING.md
View File

@@ -29,6 +29,26 @@ change becomes a place where a bug may have been introduced. Consider splitting
reformatting changes into a separate PR from those that make a behavioral change, as the tests help
guarantee that the behavior is unchanged.
Guidelines for PR size:
- Aim for PRs under 500 lines of changes when possible.
- Split large features into incremental PRs that build on each other.
- Separate refactoring, formatting, and functional changes into different PRs.
- If a large PR is unavoidable, clearly explain why in the PR description.
### Breaking changes and backwards compatibility
We prioritize maintaining backwards compatibility and minimizing disruption to users:
- **Prefer deprecation over removal**: Add deprecation warnings rather than immediately removing
public APIs
- **Provide migration paths**: Include clear upgrade instructions for any breaking changes
- **Follow our deprecation policy**: Wait at least two versions before removing deprecated items
- **Consider feature flags**: Use feature flags for experimental or potentially disruptive changes
- **Document breaking changes**: Clearly mark breaking changes in commit messages and PR descriptions
See our [deprecation notice policy](#deprecation-notice) for more details.
### Code formatting
Run `cargo xtask format` before committing to ensure that code is consistently formatted with
@@ -41,6 +61,11 @@ to be installed when running rustfmt. You can install the nightly version of Rus
rustup install nightly
```
```suggestion
> [!IMPORTANT]
> Do not modify formatting configuration (`rustfmt.toml`, `.clippy.toml`) without
> prior discussion. These changes affect all contributors and should be carefully considered.
### Search `tui-rs` for similar work
The original fork of Ratatui, [`tui-rs`](https://github.com/fdehau/tui-rs/), has a large amount of
@@ -67,6 +92,15 @@ Running `cargo xtask ci` before pushing will perform the same checks that we do
It's not mandatory to do this before pushing, however it may save you time to do so instead of
waiting for GitHub to run the checks.
Available xtask commands:
- `cargo xtask ci` - Run all CI checks
- `cargo xtask format` - Format code
- `cargo xtask lint` - Run linting checks
- `cargo xtask test` - Run all tests
Run `cargo xtask --help` to see all available commands.
### Sign your commits
We use commit signature verification, which will block commits from being merged via the UI unless
@@ -74,6 +108,17 @@ they are signed. To set up your machine to sign commits, see [managing commit si
verification](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)
in GitHub docs.
### Configuration and build system changes
Changes to project configuration files require special consideration:
- Linting configuration (`.clippy.toml`, `rustfmt.toml`): Affects all contributors.
- CI configuration (`.github/workflows/`): Affects build and deployment.
- Build system (`xtask/`, `Cargo.toml` workspace config): Affects development workflow.
- Dependencies: Consider MSRV compatibility and licensing.
Please discuss these changes in an issue before implementing them.
## Implementation Guidelines
### Setup
@@ -97,7 +142,13 @@ For an understanding of the crate organization and design decisions, see [ARCHIT
document explains the modular workspace structure introduced in version 0.30.0 and provides
guidance on which crate to use for different use cases.
[ARCHITECTURE.md]: ./ARCHITECTURE.md
When making changes, consider:
- Which crate should contain your changes per the modular structure,
- Whether your changes affect the public API of `ratatui-core` (requires extra care),
- And how your changes fit into the overall architecture.
[ARCHITECTURE.md]: https://github.com/ratatui/ratatui/blob/main/ARCHITECTURE.md
### Tests
@@ -115,6 +166,10 @@ If an area that you're making a change in is not tested, write tests to characte
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`).
> [!IMPORTANT]
> Do not remove existing tests without clear justification. If tests need to be
> modified due to API changes, explain why in your PR description.
For coverage, we have two [bacon](https://dystroy.org/bacon/) jobs (one for all tests, and one for
unit tests, keyboard shortcuts `v` and `u` respectively) that run
[cargo-llvm-cov](https://github.com/taiki-e/cargo-llvm-cov) to report the coverage. Several plugins
@@ -182,6 +237,14 @@ We generally want to wait at least two versions before removing deprecated items
time to update. However, if a deprecation is blocking for us to implement a new feature we may
*consider* removing it in a one version notice.
Deprecation process:
1. Add `#[deprecated]` attribute with a clear message.
2. Update documentation to point to the replacement.
3. Add an entry to `BREAKING-CHANGES.md` if applicable.
4. Wait at least two versions before removal.
5. Consider the impact on the ecosystem before removing.
### Use of unsafe for optimization purposes
We don't currently use any unsafe code in Ratatui, and would like to keep it that way. However, there