M-Labs
/
artiq-zynq
10
1
Fork 19
Code Issues 13 Pull Requests Releases Wiki Activity

pre-commit-hooks #329

Merged
sb10q merged 8 commits from newell/artiq-zynq:pre-commit-hooks into master 2024-10-08 15:19:13 +08:00
Conversation 0 Commits 8 Files Changed 5 +76
5 changed files with 76 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
README.md
View File

@ -59,6 +59,16 @@ Notes:
- Firmware type must be either ``runtime`` for DRTIO-less or DRTIO master variants, or ``satman`` for DRTIO satellite. - Firmware type must be either ``runtime`` for DRTIO-less or DRTIO master variants, or ``satman`` for DRTIO satellite.
- If the board is connected to the local machine, use the ``local_run.sh`` script. - If the board is connected to the local machine, use the ``local_run.sh`` script.
Pre-Commit Hooks
----------------
You are strongly recommended to use the provided pre-commit hooks to automatically reformat files and check for non-optimal Rust/C/C++ practices. Run `pre-commit install` to install the hook and `pre-commit` will automatically run `cargo fmt`, `cargo clippy`, and `clang-format` for you.
Several things to note:
- If `cargo fmt`, `cargo clippy`, or `clang-format` returns an error, the pre-commit hook will fail. You should fix all errors before trying to commit again.
- If `cargo fmt` or `clang-format` reformats some files, the pre-commit hook will also fail. You should review the changes and, if satisfied, try to commit again.
License License
------- -------

1
flake.nix
View File

@ -384,6 +384,7 @@
artiqpkgs.artiq artiqpkgs.artiq
artiqpkgs.vivado artiqpkgs.vivado
binutils-arm binutils-arm
pre-commit
]; ];
XARGO_RUST_SRC = "${rust}/lib/rustlib/src/rust/library"; XARGO_RUST_SRC = "${rust}/lib/rustlib/src/rust/library";
CLANG_EXTRA_INCLUDE_DIR = "${llvmPackages_11.clang-unwrapped.lib}/lib/clang/11.1.0/include"; CLANG_EXTRA_INCLUDE_DIR = "${llvmPackages_11.clang-unwrapped.lib}/lib/clang/11.1.0/include";

32
src/.clang-format Normal file
View File

@ -0,0 +1,32 @@
BasedOnStyle: LLVM
newell marked this conversation as resolved
esavkin commented 2024-10-07 10:13:41 +08:00
Review
Copy Link

Isn't rustfmt.toml enough already?

Isn't rustfmt.toml enough already?
newell commented 2024-10-07 10:35:20 +08:00
Review
Copy Link

I don't know. I was modeling this branch after what nac3 does with its pre-commit hooks but there could very well be some parameters that should be changed (such as removing the line of code you are commenting on). Open to suggestions.

I don't know. I was modeling this branch after what `nac3` does with its pre-commit hooks but there could very well be some parameters that should be changed (such as removing the line of code you are commenting on). Open to suggestions.
sb10q commented 2024-10-07 10:58:17 +08:00
Review
Copy Link

@esavkin Pay attention to the file name and its contents ("Language: Cpp"...), this is about Clang/C/C++ not Rust.

That being said I don't see this clang formatter being applied in the precommit hook, neither here nor in nac3. @derppening

@esavkin Pay attention to the file name and its contents ("Language: Cpp"...), this is about Clang/C/C++ not Rust. That being said I don't see this clang formatter being applied in the precommit hook, neither here nor in nac3. @derppening
sb10q commented 2024-10-07 10:59:03 +08:00
Review
Copy Link

Also does it work with pure C code like https://git.m-labs.hk/M-Labs/artiq-zynq/src/branch/master/src/libc/src/printf.c ?

Also does it work with pure C code like https://git.m-labs.hk/M-Labs/artiq-zynq/src/branch/master/src/libc/src/printf.c ?
newell commented 2024-10-07 11:33:34 +08:00
Review
Copy Link

Also does it work with pure C code like https://git.m-labs.hk/M-Labs/artiq-zynq/src/branch/master/src/libc/src/printf.c ?

Just tested with the C code and it doesn't catch it:

nac3 cargo format....................................(no files to check)Skipped
nac3 cargo clippy....................................(no files to check)Skipped
[pre-commit-hooks b7054d0] testing
 1 file changed, 3 insertions(+), 3 deletions(-)

NOTE: I did test with Rust code before creating the pull request and it does catch that.

> Also does it work with pure C code like https://git.m-labs.hk/M-Labs/artiq-zynq/src/branch/master/src/libc/src/printf.c ? Just tested with the C code and it doesn't catch it: ``` nac3 cargo format....................................(no files to check)Skipped nac3 cargo clippy....................................(no files to check)Skipped [pre-commit-hooks b7054d0] testing 1 file changed, 3 insertions(+), 3 deletions(-) ``` NOTE: I did test with Rust code before creating the pull request and it does catch that.
derppening commented 2024-10-07 12:16:18 +08:00
Review
Copy Link

@esavkin Pay attention to the file name and its contents ("Language: Cpp"...), this is about Clang/C/C++ not Rust.

That being said I don't see this clang formatter being applied in the precommit hook, neither here nor in nac3. @derppening

It's not in NAC3 at the moment. We should enable it at some point though.

> @esavkin Pay attention to the file name and its contents ("Language: Cpp"...), this is about Clang/C/C++ not Rust. > > That being said I don't see this clang formatter being applied in the precommit hook, neither here nor in nac3. @derppening It's not in NAC3 at the moment. We should enable it at some point though.
newell commented 2024-10-08 01:21:04 +08:00
Review
Copy Link

I updated the branch with a clang-format hook.

I updated the branch with a clang-format hook.
👍 1
Language: Cpp
Standard: Cpp11
AccessModifierOffset: -1
AlignEscapedNewlines: Left
AlwaysBreakAfterReturnType: None
AlwaysBreakTemplateDeclarations: Yes
AllowAllParametersOfDeclarationOnNextLine: false
AllowShortFunctionsOnASingleLine: Inline
BinPackParameters: false
BreakBeforeBinaryOperators: NonAssignment
BreakBeforeTernaryOperators: true
BreakConstructorInitializers: AfterColon
BreakInheritanceList: AfterColon
ColumnLimit: 120
ConstructorInitializerAllOnOneLineOrOnePerLine: true
ContinuationIndentWidth: 4
DerivePointerAlignment: false
IndentCaseLabels: true
IndentPPDirectives: None
IndentWidth: 4
MaxEmptyLinesToKeep: 1
PointerAlignment: Left
ReflowComments: true
SortIncludes: false
SortUsingDeclarations: true
SpaceAfterTemplateKeyword: false
SpacesBeforeTrailingComments: 2
TabWidth: 4
UseTab: Never

1
src/.clippy.toml Normal file
View File

@ -0,0 +1 @@
doc-valid-idents = ["CPython", "NumPy", ".."]

32
src/.pre-commit-config.yaml Normal file
View File

@ -0,0 +1,32 @@
# See https://pre-commit.com for more information
# See https://pre-commit.com/hooks.html for more hooks
default_stages: [commit]
repos:
- repo: local
hooks:
- id: cargo-fmt
name: artiq-zynq cargo format
entry: nix
language: system
types: [file, rust]
pass_filenames: false
description: Runs cargo fmt on the codebase.
args: [develop, -c, cargo, fmt, --manifest-path, src/Cargo.toml, --all]
- id: cargo-clippy
name: artiq-zynq cargo clippy
entry: nix
language: system
types: [file, rust]
pass_filenames: false
description: Runs cargo clippy on the codebase.
args: [develop, -c, cargo, clippy, --manifest-path, src/Cargo.toml, --tests]
- repo: https://github.com/pre-commit/mirrors-clang-format
rev: v19.1.0
hooks:
- id: clang-format
name: artiq-zynq clang-format
description: Runs clang-format on the codebase.
files: \.(cpp|h|hpp|c)$
args: [-style=file, -fallback-style=none, -assume-filename=src/.clang-format]