From 67c261c3eeefaa18e67b5c4a50c9ff5e70d656cc Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Wed, 12 Jun 2024 13:38:31 +0200 Subject: [PATCH 1/6] Fixed documentation errors --- miden-tx/src/auth/tx_authenticator.rs | 2 +- objects/src/accounts/storage/map.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/miden-tx/src/auth/tx_authenticator.rs b/miden-tx/src/auth/tx_authenticator.rs index ddf52abd4..28a254e25 100644 --- a/miden-tx/src/auth/tx_authenticator.rs +++ b/miden-tx/src/auth/tx_authenticator.rs @@ -42,7 +42,7 @@ pub trait TransactionAuthenticator { // ================================================================================================ #[derive(Clone, Debug)] -/// Represents a signer for [AuthSecretKey] keys +/// Represents a signer for [AuthSecretKey] keys. pub struct BasicAuthenticator { /// pub_key |-> secret_key mapping keys: BTreeMap, diff --git a/objects/src/accounts/storage/map.rs b/objects/src/accounts/storage/map.rs index 276a9c3b5..491645dc9 100644 --- a/objects/src/accounts/storage/map.rs +++ b/objects/src/accounts/storage/map.rs @@ -46,7 +46,7 @@ impl StorageMap { /// Returns a new [StorageMap]. /// - /// All leaves in the returned tree are set to [Self::EMPTY_VALUE]. + /// All leaves in the returned tree are set to \[Self::EMPTY_VALUE\]. pub fn new() -> Self { StorageMap { map: Smt::new() } } From 51ad525acbd93eaa97e397ff33344ad4255447a5 Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Wed, 12 Jun 2024 15:17:11 +0200 Subject: [PATCH 2/6] updated pre-commit + add docs + add make check --- .pre-commit-config.yaml | 74 +++++++++++++++++------------------------ Makefile | 6 ++++ README.md | 4 +++ 3 files changed, 41 insertions(+), 43 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b76674d71..3d35fb2b6 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,46 +1,34 @@ # See https://pre-commit.com for more information # See https://pre-commit.com/hooks.html for more hooks repos: -- repo: https://github.com/pre-commit/pre-commit-hooks - rev: v3.2.0 - hooks: - - id: trailing-whitespace - - id: end-of-file-fixer - - id: check-yaml - - id: check-json - - id: check-toml - - id: pretty-format-json - - id: check-added-large-files - - id: check-case-conflict - - id: check-executables-have-shebangs - - id: check-merge-conflict - - id: detect-private-key -- repo: https://github.com/hackaugusto/pre-commit-cargo - rev: v1.0.0 - hooks: - # Allows cargo fmt to modify the source code prior to the commit - - id: cargo - name: Cargo fmt - args: ["+nightly", "fmt", "--all"] - stages: [commit] - # Requires code to be properly formatted prior to pushing upstream - - id: cargo - name: Cargo fmt --check - args: ["+nightly", "fmt", "--all", "--check"] - stages: [push, manual] - - id: cargo - name: Cargo check --all-targets - args: ["+stable", "check", "--all-targets"] - - id: cargo - name: Cargo check --all-targets --no-default-features - args: ["+stable", "check", "--no-default-features", "--workspace"] - - id: cargo - name: Cargo check --all-targets --all-features - args: ["+stable", "check", "--all-targets", "--all-features", "--workspace"] - # Unlike fmt, clippy will not be automatically applied - - id: cargo - name: Cargo clippy - args: ["+nightly", "clippy", "--workspace", "--all-targets", "--", "--deny", "clippy::all", "--deny", "warnings"] - - id: cargo - name: Cargo clippy all-features - args: ["+nightly", "clippy", "--workspace", "--all-targets", "--all-features", "--", "--deny", "clippy::all", "--deny", "warnings"] + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.6.0 + hooks: + - id: trailing-whitespace + - id: end-of-file-fixer + - id: check-yaml + - id: check-json + - id: check-toml + - id: pretty-format-json + - id: check-added-large-files + - id: check-case-conflict + - id: check-executables-have-shebangs + - id: check-merge-conflict + - id: detect-private-key + - repo: local + hooks: + - id: lint + name: Make lint + stages: [commit] + language: rust + entry: make lint + - id: doc + name: Make doc + stages: [commit] + language: rust + entry: make doc + - id: check + name: Make check + stages: [commit] + language: rust + entry: make check diff --git a/Makefile b/Makefile index e3584a302..4a164b8d0 100644 --- a/Makefile +++ b/Makefile @@ -61,6 +61,12 @@ test-prove: ## Run `prove` tests (tests which use the Miden prover) .PHONY: test test: test-default test-prove ## Run all tests +# --- checking ------------------------------------------------------------------------------------ + +.PHONY: check +check: ## Check all targets and features + cargo check --all-targets $(ALL_FEATURES_BUT_ASYNC) + # --- building ------------------------------------------------------------------------------------ .PHONY: build diff --git a/README.md b/README.md index d94531713..d6007c3c6 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,10 @@ make test Some of the functions in this project are computationally intensive and may take a significant amount of time to compile and complete during testing. To ensure optimal results we use the `make test` command. It enables the running of tests in release mode and using specific configurations replicates the test conditions of the development mode and verifies all debug assertions. +## Commiting + +To make sure all commits adhere to our programming standards we use [pre-commit](https://pre-commit.com/) ([file](.pre-commit-config.yaml)) to run automatic commands on each commit. Please install it and follow the setup instructions for your machine. + ## License This project is [MIT licensed](./LICENSE) From 492b7428be7decb65f70ac5f6a6c09812949d0c4 Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Wed, 12 Jun 2024 15:24:21 +0200 Subject: [PATCH 3/6] Updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b1ce5fd3c..474805c24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Enhancements +- Updated and improved [.pre-commit-config.yaml](.pre-commit-config.yaml) file (#748) - Fixed documentation and added `make doc` CI job (#746) - Made `DataStore` conditionally async using `winter-maybe-async` (#725) - Add `Option`to `NoteFile` (#741). From df670301ca0b2e75fec9e39f40227dfda43df793 Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Thu, 13 Jun 2024 12:08:19 +0200 Subject: [PATCH 4/6] Added CONTRIBUTING.md, improved comment on make check --- CONTRIBUTING.md | 128 ++++++++++++++++++++++++++++++++++++++++++++++++ Makefile | 2 +- README.md | 4 -- 3 files changed, 129 insertions(+), 5 deletions(-) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 000000000..253b3abf4 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,128 @@ +# Contributing to Miden Base + +#### First off, thanks for taking the time to contribute! + +We want to make contributing to this project as easy and transparent as possible, whether it's: + +- Reporting a [bug](https://github.com/0xPolygonMiden/miden-base/issues/new?assignees=&labels=bug&projects=&template=1-bugreport.yml) +- Taking part in [discussions](https://github.com/0xPolygonMiden/miden-base/discussions) +- Submitting a [fix](https://github.com/0xPolygonMiden/miden-base/pulls) +- Proposing new [features](https://github.com/0xPolygonMiden/miden-base/issues/new?assignees=&labels=enhancement&projects=&template=2-feature-request.yml) + +  + +## Flow + +We are using [Github Flow](https://docs.github.com/en/get-started/quickstart/github-flow), so all code changes happen through pull requests from a [forked repo](https://docs.github.com/en/get-started/quickstart/fork-a-repo). + +### Branching + +- The current active branch is `next`. Every branch with a fix/feature must be forked from `next`. + +- The branch name should contain a short issue/feature description separated with hyphens [(kebab-case)](https://en.wikipedia.org/wiki/Letter_case#Kebab_case). + + For example, if the issue title is `Fix functionality X in component Y` then the branch name will be something like: `fix-x-in-y`. + +- New branch should be rebased from `next` before submitting a PR in case there have been changes to avoid merge commits. + i.e. this branches state: + + ``` + A---B---C fix-x-in-y + / + D---E---F---G next + | | + (F, G) changes happened after `fix-x-in-y` forked + ``` + + should become this after rebase: + + ``` + A'--B'--C' fix-x-in-y + / + D---E---F---G next + ``` + + More about rebase [here](https://git-scm.com/docs/git-rebase) and [here](https://www.atlassian.com/git/tutorials/rewriting-history/git-rebase#:~:text=What%20is%20git%20rebase%3F,of%20a%20feature%20branching%20workflow.) + +### Commit messages + +- Commit messages should be written in a short, descriptive manner and be prefixed with tags for the change type and scope (if possible) according to the [semantic commit](https://gist.github.com/joshbuchea/6f47e86d2510bce28f8e7f42ae84c716) scheme. + For example, a new change to the `miden-node-store` crate might have the following message: `feat(miden-node-store): fix block-headers database schema` + +- Also squash commits to logically separated, distinguishable stages to keep git log clean: + + ``` + 7hgf8978g9... Added A to X \ + \ (squash) + gh354354gh... oops, typo --- * ---------> 9fh1f51gh7... feat(X): add A && B + / + 85493g2458... Added B to X / + + + 789fdfffdf... Fixed D in Y \ + \ (squash) + 787g8fgf78... blah blah --- * ---------> 4070df6f00... fix(Y): fixed D && C + / + 9080gf6567... Fixed C in Y / + ``` + +### Code Style and Documentation + +- For documentation in the codebase, we follow the [rustdoc](https://doc.rust-lang.org/rust-by-example/meta/doc.html) convention with no more than 100 characters per line. +- For code sections, we use code separators like the following to a width of 100 characters:: + + ``` + // CODE SECTION HEADER + // ================================================================================ + ``` + +- [Rustfmt](https://github.com/rust-lang/rustfmt), [Clippy](https://github.com/rust-lang/rust-clippy) and [Rustdoc](https://doc.rust-lang.org/rustdoc/index.html) linting is included in CI pipeline. Anyways it's preferable to run linting locally before push. To simplify running these commands in a reproducible manner we use [cargo-make](https://github.com/sagiegurari/cargo-make), you can run: + + ``` + cargo make lint + ``` + +You can find more information about the `cargo make` commands in the [Makefile](Makefile.toml) + +### Testing + +After writing code different types of tests (unit, integration, end-to-end) are required to make sure that the correct behavior has been achieved and that no bugs have been introduced. You can run tests using the following command: + +``` +cargo make test +``` + +### Versioning + +We use [semver](https://semver.org/) naming convention. + +  + +## Pre-PR checklist + +To make sure all commits adhere to our programming standards we use [pre-commit](https://pre-commit.com/) ([file](.pre-commit-config.yaml)) to run automatic commands on each commit. Please install it and follow the setup instructions for your machine. + +1. Repo forked and branch created from `next` according to the naming convention. +2. Commit messages and code style follow conventions. +3. Tests added for new functionality. +4. Documentation/comments updated for all changes according to our documentation convention. +5. Rustfmt, Clippy and Rustdoc linting passed (Will be ran automatically by pre-commit). +6. New branch rebased from `next`. + +  + +## Write bug reports with detail, background, and sample code + +**Great Bug Reports** tend to have: + +- A quick summary and/or background +- Steps to reproduce +- What you expected would happen +- What actually happens +- Notes (possibly including why you think this might be happening, or stuff you tried that didn't work) + +  + +## Any contributions you make will be under the MIT Software License + +In short, when you submit code changes, your submissions are understood to be under the same [MIT License](http://choosealicense.com/licenses/mit/) that covers the project. Feel free to contact the maintainers if that's a concern. diff --git a/Makefile b/Makefile index 4a164b8d0..87e26b19a 100644 --- a/Makefile +++ b/Makefile @@ -64,7 +64,7 @@ test: test-default test-prove ## Run all tests # --- checking ------------------------------------------------------------------------------------ .PHONY: check -check: ## Check all targets and features +check: ## Check all targets and features for errors without code generation cargo check --all-targets $(ALL_FEATURES_BUT_ASYNC) # --- building ------------------------------------------------------------------------------------ diff --git a/README.md b/README.md index d6007c3c6..d94531713 100644 --- a/README.md +++ b/README.md @@ -70,10 +70,6 @@ make test Some of the functions in this project are computationally intensive and may take a significant amount of time to compile and complete during testing. To ensure optimal results we use the `make test` command. It enables the running of tests in release mode and using specific configurations replicates the test conditions of the development mode and verifies all debug assertions. -## Commiting - -To make sure all commits adhere to our programming standards we use [pre-commit](https://pre-commit.com/) ([file](.pre-commit-config.yaml)) to run automatic commands on each commit. Please install it and follow the setup instructions for your machine. - ## License This project is [MIT licensed](./LICENSE) From 484ab5109cb593db623f1f19062e5cbb33850cf7 Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Thu, 13 Jun 2024 12:10:41 +0200 Subject: [PATCH 5/6] Fix typo --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 253b3abf4..325f5ef53 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -47,7 +47,7 @@ We are using [Github Flow](https://docs.github.com/en/get-started/quickstart/git ### Commit messages - Commit messages should be written in a short, descriptive manner and be prefixed with tags for the change type and scope (if possible) according to the [semantic commit](https://gist.github.com/joshbuchea/6f47e86d2510bce28f8e7f42ae84c716) scheme. - For example, a new change to the `miden-node-store` crate might have the following message: `feat(miden-node-store): fix block-headers database schema` + For example, a new change to the `miden-objects` crate might have the following message: `feat(miden-objects): Added Account deserialization` - Also squash commits to logically separated, distinguishable stages to keep git log clean: From 65652972ccbb81dd5e4c00c052fa57b231b4e798 Mon Sep 17 00:00:00 2001 From: Bobbin Threadbare Date: Thu, 13 Jun 2024 11:18:00 -0700 Subject: [PATCH 6/6] docs: minor changes in CONTRIBUTING.md --- CONTRIBUTING.md | 8 ++++---- objects/src/accounts/storage/map.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 325f5ef53..0d676a167 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -76,20 +76,20 @@ We are using [Github Flow](https://docs.github.com/en/get-started/quickstart/git // ================================================================================ ``` -- [Rustfmt](https://github.com/rust-lang/rustfmt), [Clippy](https://github.com/rust-lang/rust-clippy) and [Rustdoc](https://doc.rust-lang.org/rustdoc/index.html) linting is included in CI pipeline. Anyways it's preferable to run linting locally before push. To simplify running these commands in a reproducible manner we use [cargo-make](https://github.com/sagiegurari/cargo-make), you can run: +- [Rustfmt](https://github.com/rust-lang/rustfmt), [Clippy](https://github.com/rust-lang/rust-clippy) and [Rustdoc](https://doc.rust-lang.org/rustdoc/index.html) linting is included in CI pipeline. Anyways it's preferable to run linting locally before push. To simplify running these commands in a reproducible manner we use `make` commands, you can run: ``` - cargo make lint + make lint ``` -You can find more information about the `cargo make` commands in the [Makefile](Makefile.toml) +You can find more information about the `make` commands in the [Makefile](Makefile) ### Testing After writing code different types of tests (unit, integration, end-to-end) are required to make sure that the correct behavior has been achieved and that no bugs have been introduced. You can run tests using the following command: ``` -cargo make test +make test ``` ### Versioning diff --git a/objects/src/accounts/storage/map.rs b/objects/src/accounts/storage/map.rs index 491645dc9..276a9c3b5 100644 --- a/objects/src/accounts/storage/map.rs +++ b/objects/src/accounts/storage/map.rs @@ -46,7 +46,7 @@ impl StorageMap { /// Returns a new [StorageMap]. /// - /// All leaves in the returned tree are set to \[Self::EMPTY_VALUE\]. + /// All leaves in the returned tree are set to [Self::EMPTY_VALUE]. pub fn new() -> Self { StorageMap { map: Smt::new() } }