From ee72888973071aa05e507a4cbe61c78449a64e79 Mon Sep 17 00:00:00 2001 From: Yuki Kishimoto Date: Wed, 10 Apr 2024 11:27:01 +0200 Subject: [PATCH] doc: update CONTRIBUTING.md` --- CODE_STYLE.md | 187 ------------------------------------------------ CONTRIBUTING.md | 100 +------------------------- 2 files changed, 1 insertion(+), 286 deletions(-) delete mode 100644 CODE_STYLE.md diff --git a/CODE_STYLE.md b/CODE_STYLE.md deleted file mode 100644 index 2406215..0000000 --- a/CODE_STYLE.md +++ /dev/null @@ -1,187 +0,0 @@ -# Code style - -This is a description of a coding style that every contributor **must** follow. -Please, read the whole document before you start pushing code. - -## Generics - -All trait bounds should be written in `where`: - -```rust -// GOOD -pub fn new(user_id: i32, name: N, title: T, png_sticker: P, emojis: E) -> Self -where - N: Into, - T: Into, - P: Into, - E: Into, -{ ... } - -// BAD -pub fn new, - T: Into, - P: Into, - E: Into> - (user_id: i32, name: N, title: T, png_sticker: P, emojis: E) -> Self { ... } -``` - -```rust -// GOOD -impl Trait for Wrap -where - T: Trait -{ ... } - -// BAD -impl Trait for Wrap { ... } -``` - -## Use `Self` where possible - -When referring to the type for which block is implemented, prefer using `Self`, rather than the name of the type: - -```rust -impl ErrorKind { - // GOOD - fn print(&self) { - Self::Io => println!("Io"), - Self::Network => println!("Network"), - Self::Json => println!("Json"), - } - - // BAD - fn print(&self) { - ErrorKind::Io => println!("Io"), - ErrorKind::Network => println!("Network"), - ErrorKind::Json => println!("Json"), - } -} -``` - -```rust -impl<'a> AnswerCallbackQuery<'a> { - // GOOD - fn new(bot: &'a Bot, callback_query_id: C) -> Self - where - C: Into, - { ... } - - // BAD - fn new(bot: &'a Bot, callback_query_id: C) -> AnswerCallbackQuery<'a> - where - C: Into, - { ... } -} -``` - -**Rationale:** `Self` is generally shorter and it's easier to copy-paste code or rename the type. - -## Deriving traits (apply only to libraries) - -Derive `Debug`, `Clone`, `Copy`, `PartialEq`, `Eq` and `Hash` for public types when possible (in this order). - -**Rationale:** these traits can be useful for users and can be implemented for most types. - -Derive `Default` when there is a reasonable default value for the type. - -## Full paths for logging - -Always write `tracing::!(...)` instead of importing `use tracing::;` and invoking `!(...)`. - -```rust -// GOOD -tracing::warn!("Everything is on fire"); - -// BAD -use tracing::warn; - -warn!("Everything is on fire"); -``` - -**Rationale:** -- Less polluted import blocks -- Uniformity - -## `&str` -> `String` conversion - -Prefer using `.to_string()` or `.to_owned()`, rather than `.into()`, `String::from`, etc. - -**Rationale:** uniformity, intent clarity. - -## Order of imports - -```rust -// First core, alloc and/or std -use core::fmt; -use std::{...}; - -// Second, external crates (both crates.io crates and other rust-analyzer crates). -use crate_foo::{ ... }; -use crate_bar::{ ... }; - -// If applicable, the current sub-modules -mod x; -mod y; - -// Finally, the internal crate modules and submodules -use crate::{}; -use super::{}; - -// Re-exports are treated as item definitions rather than imports, so they go -// after imports and modules. Use them sparingly. -pub use crate::x::Z; -``` - -## Import Style - -When implementing traits from `core::fmt`/`std::fmt` import the module: - -```rust -// GOOD -use core::fmt; - -impl fmt::Display for RenameError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { .. } -} - -// BAD -impl core::fmt::Display for RenameError { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { .. } -} -``` - -## If-let - -Avoid the `if let ... { } else { }` construct if possible, use `match` instead: - -```rust -// GOOD -match ctx.expected_type.as_ref() { - Some(expected_type) => completion_ty == expected_type && !expected_type.is_unit(), - None => false, -} - -// BAD -if let Some(expected_type) = ctx.expected_type.as_ref() { - completion_ty == expected_type && !expected_type.is_unit() -} else { - false -} -``` - -Use `if let ... { }` when a match arm is intentionally empty: - -```rust -// GOOD -if let Some(expected_type) = this.as_ref() { - // Handle it -} - -// BAD -match this.as_ref() { - Some(expected_type) => { - // Handle it - }, - None => (), -} -``` \ No newline at end of file diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6e0613f..fbf0449 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,101 +1,3 @@ # Contributing -## Contribution Workflow - -To contribute a patch: - -* Fork Repository -* Create topic branch -* Commit patches (PR, emails, ...) - -In general commits should be atomic and diffs **should be easy to read**. - -## Code style guide - -Before writing code, please read [our code style](./CODE_STYLE.md). - -## Commit format - -The commit **must** be formatted in the following way: - -``` -: - - -``` - -If applicable, link the `issue`/`PR` to be closed with: - -* Closes -* Fixes - -The `context` name should be: - -* `nostr` if changes are related to the main Rust `nostr` crate (or `protocol`?) -* `sdk`, `cli`, `pool`, `signer`, `nwc` and so on for the others Rust crates (so basically remove the `nostr-` prefix) -* `ffi()` for `UniFFI` and `js()` for `JavaScript` bindings (follow same above rules for the ``) -* `book` if changes are related to the `book` -* `doc` for the `.md` files (except for `CHANGELOG.md`?) - -Anything that haven't a specific context, can be left without the `:` prefix (ex. change to main `justfile` commands, change to `CHANGELOG.md`?) - -### Examples - -``` -nostr: add NIP32 support - -Added kinds, tags and EventBuilder constructors to support NIP32. - -Closes https://.com/rust-nostr/nostr/issue/1234 -``` - -``` -pool: fix connection issues - -Long description... - -Fixes https://.com/rust-nostr/nostr/issue/5612 -``` - -``` -nwc: add `pay_multiple_invoices` support - -Closes https://.com/rust-nostr/nostr/issue/2222 -``` - -``` -ffi(nostr): add `EventBuilder::mute_list` -``` - -``` -ffi(sdk): add `AbortHandle` - -* Return `AbortHandle` in `Client::handle_notifications` -* Another change... -``` - -``` -js(sdk): replace log `file path` with `module path` -``` - -## Deprecation policy - -Where possible, breaking existing APIs should be avoided. Instead, add new APIs and -use [`#[deprecated]`](https://github.com/rust-lang/rfcs/blob/master/text/1270-deprecation.md) -to discourage use of the old one. - -Deprecated APIs are typically maintained for one release cycle. In other words, an -API that has been deprecated with the 0.10 release can be expected to be removed in the -0.11 release. This allows for smoother upgrades without incurring too much technical -debt inside this library. - -If you deprecated an API as part of a contribution, we encourage you to "own" that API -and send a follow-up to remove it as part of the next release cycle. - -## Unwrap and expect - -Usage of `.unwrap()` or `.expect("...")` methods is allowed **only** in `examples` or `tests`. - -## Coding Conventions - -Use `just precommit` or `just check` to format and check the code before committing. This is also enforced by the CI. +Check `CONTRIBUTING.md` and `CODE_STYLE.md` at https://github.com/rust-nostr/nostr