M-Labs/artiq-zynq
10
1
Fork 19
Code Issues 12 Pull Requests 1 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
newell commented 2024-10-06 08:49:12 +08:00
Contributor
Copy Link

Add pre-commit hooks to artiq-zynq.

Add `pre-commit` hooks to `artiq-zynq`.
newell added 3 commits 2024-10-06 08:49:13 +08:00
esavkin reviewed 2024-10-07 10:13:41 +08:00
src/.clang-format
@ -0,0 +1,32 @@
BasedOnStyle: LLVM
esavkin commented 2024-10-07 10:13:41 +08:00
Owner
Copy Link

Isn't rustfmt.toml enough already?

Isn't rustfmt.toml enough already?
newell commented 2024-10-07 10:35:20 +08:00
Author
Contributor
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
Owner
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
Owner
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
Author
Contributor
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
Collaborator
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
Author
Contributor
Copy Link

I updated the branch with a clang-format hook.

I updated the branch with a clang-format hook.
👍 1
newell marked this conversation as resolved
newell added 1 commit 2024-10-08 01:00:46 +08:00
newell added 1 commit 2024-10-08 01:03:02 +08:00
newell added 1 commit 2024-10-08 01:24:29 +08:00
sb10q requested review from derppening 2024-10-08 09:07:09 +08:00
derppening approved these changes 2024-10-08 12:05:41 +08:00
sb10q reviewed 2024-10-08 14:00:21 +08:00
src/.pre-commit-config.yaml Outdated
@ -0,0 +6,4 @@
repos:
- repo: local
hooks:
- id: nac3-cargo-fmt
sb10q commented 2024-10-08 14:00:21 +08:00
Owner
Copy Link

nac3?

nac3?
newell marked this conversation as resolved
newell added 1 commit 2024-10-08 14:12:41 +08:00
newell added 1 commit 2024-10-08 14:15:45 +08:00
sb10q merged commit 030247be18 into master 2024-10-08 15:19:12 +08:00
Sign in to join this conversation.
Reviewers
No reviewers
derppening
Labels
Clear labels   
priority:critical

   
priority:high

   
priority:low

   
priority:medium
No Label
priority:critical
priority:high
priority:low
priority:medium
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/artiq-zynq#329
No description provided.
Delete Branch "newell/artiq-zynq:pre-commit-hooks"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?