public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>, Stefan Lendl <s.lendl@proxmox.com>
Subject: Re: [pbs-devel] [RFC v2 proxmox-backup] git-hooks: pre-commit runs cargo fmt --check
Date: Mon, 15 Jan 2024 08:27:31 +0100	[thread overview]
Message-ID: <e4cac481-c0ba-4092-9e2d-458e7aaac290@proxmox.com> (raw)
In-Reply-To: <20240112144328.353398-1-s.lendl@proxmox.com>

Am 12/01/2024 um 15:43 schrieb Stefan Lendl:
> * add a pre-commit hook that declines commiting if cargo fmt would make

as said off-list I do not want anything that blocks committing,
that is a PITA in development especially as this doesn't gains us
much... so NAK!

Let's rather add a fmt --check test to the buildbot, then we get
pinged on issues and a maintainer can just commit and push a cargo
fmt run without that much fuzz.. 

>   formatting changes.
> * `make install-git-hook` installs the hook
> * `make build` installs the hook
> * `make uninstall-git-hook` removes the hook
> * `make lint` runs cargo fmt --check in addition to cargo clippy
> 
> Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
> Suggested-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> 
> Notes:
>     Changes from v1:
>     
>     * Fix indentation style in hook *sigh*
> 
>  Makefile             |  9 ++++++++-
>  git-hooks/pre-commit | 13 +++++++++++++
>  2 files changed, 21 insertions(+), 1 deletion(-)
>  create mode 100755 git-hooks/pre-commit
> 
> diff --git a/Makefile b/Makefile
> index 0317dd5e..fc866872 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -86,7 +86,7 @@ doc:
>  
>  # always re-create this dir
>  .PHONY: build
> -build:
> +build: install-git-hooks
>  	rm -rf build
>  	mkdir build
>  	git rev-parse HEAD > build/.repoid
> @@ -188,6 +188,7 @@ $(COMPILED_BINS) $(COMPILEDIR)/dump-catalog-shell-cli $(COMPILEDIR)/docgen: .do-
>  
>  .PHONY: lint
>  lint:
> +	cargo fmt --all -- --check
>  	cargo clippy -- -A clippy::all -D clippy::correctness
>  
>  install: $(COMPILED_BINS)
> @@ -221,3 +222,9 @@ upload: $(SERVER_DEB) $(CLIENT_DEB) $(RESTORE_DEB) $(DOC_DEB)
>  	  | ssh -X repoman@repo.proxmox.com upload --product pbs --dist $(UPLOAD_DIST)
>  	tar cf - $(CLIENT_DEB) $(CLIENT_DBG_DEB) | ssh -X repoman@repo.proxmox.com upload --product "pve,pmg,pbs-client" --dist $(UPLOAD_DIST)
>  	tar cf - $(RESTORE_DEB) $(RESTORE_DBG_DEB) | ssh -X repoman@repo.proxmox.com upload --product "pve" --dist $(UPLOAD_DIST)
> +
> +install-git-hooks:
> +	ln -sn ../../git-hooks/pre-commit .git/hooks/pre-commit > /dev/null 2>&1 || true
> +
> +uninstall-git-hooks:
> +	rm -f .git/hooks/pre-commit

this is pretty intrusive and destroys other hooks people have..

> diff --git a/git-hooks/pre-commit b/git-hooks/pre-commit
> new file mode 100755
> index 00000000..6cd0b3bf
> --- /dev/null
> +++ b/git-hooks/pre-commit
> @@ -0,0 +1,13 @@
> +#!/usr/bin/env bash
> +
> +# Run cargo fmt before a commit to ensure the format style
> +
> +cargo fmt --all -- --check > /dev/null 2>&1
> +res=$?
> +
> +if [[ $res != 0 ]]; then
> +    echo "git pre-commit hook: There are some code style issues, run \`cargo fmt\` first."
> +    exit 1
> +fi
> +
> +exit 0





  reply	other threads:[~2024-01-15  7:28 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-12 14:43 Stefan Lendl
2024-01-15  7:27 ` Thomas Lamprecht [this message]
2024-01-15  9:28   ` Stefan Lendl
2024-01-15  9:42     ` Thomas Lamprecht
2024-01-15 12:46       ` Lukas Wagner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e4cac481-c0ba-4092-9e2d-458e7aaac290@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=s.lendl@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal