git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Ramsay Jones <ramsay@ramsayjones.plus.com>
Cc: "Jeff King" <peff@peff.net>,
	"Brandon Williams" <bmwill@google.com>,
	"Stefan Beller" <sbeller@google.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: [RFC] clang-format: outline the git project's coding style
Date: Sun, 13 Aug 2017 10:33:13 -0700	[thread overview]
Message-ID: <xmqqy3qnwexy.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <26bebb5b-857f-1501-5139-a5513d9875ec@ramsayjones.plus.com> (Ramsay Jones's message of "Sun, 13 Aug 2017 17:14:55 +0100")

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

> As a start, how about something like this:
>
> -- >8 --
> $ git diff
> diff --git a/Makefile b/Makefile
> index 461c845d3..7555def45 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2440,6 +2440,18 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
>  .PHONY: sparse $(SP_OBJ)
>  sparse: $(SP_OBJ)
>  
> +ST_C = $(patsubst %.o,%.stc,$(C_OBJ))
> +ST_H = $(patsubst %.h,%.sth,$(LIB_H))
> +
> +$(ST_C): %.stc: %.c FORCE
> +       checkpatch.pl --no-tree --show-types --ignore=NEW_TYPEDEFS,INLINE -f $<
> +
> +$(ST_H): %.sth: %.h FORCE
> +       checkpatch.pl --no-tree --show-types --ignore=NEW_TYPEDEFS,INLINE -f $<
> +
> +.PHONY: style $(ST_C) $(ST_H)
> +style: $(ST_C) $(ST_H)
> +
>  check: common-cmds.h
>         @if sparse; \
>         then \
> $ 
> -- >8 --
> ...
> Hmm, on reflection, it may be a bit too crude! :-D

As you already saw in the output from this, I think this is a good
illustration that shows why we want an incremental tool that works
on the changes, not on full file contents.  Contributors who want
their changes accepted and want to help the review process by
avoiding trivial coding style violations in their patches should not
have to find _their_ piece from an output about the whole file
contents, most of which is likely to have been inherited from the
original.  They are not working on Git to produce unnecessary code
churn whose only purpose is to make existing and otherwise dormant
code conform to the style tool's liking.  That's not their focus.

IOW I was expecting something that works on the output from "git
diff HEAD" or "git format-patch --stdout @{u}.."

Unless you were somehow envisioning that having a baseline line
this, and then take another full dump after their patch and
comparing the two, would make a good foundation for that incremental
checker, that is.  I am not sure if that would be a workable
approach myself, though.

Thanks.

  parent reply	other threads:[~2017-08-13 17:33 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-08  1:25 [RFC] clang-format: outline the git project's coding style Brandon Williams
2017-08-08 12:05 ` Johannes Schindelin
2017-08-08 17:40   ` Stefan Beller
2017-08-08 18:23     ` Brandon Williams
2017-08-09 22:33       ` Johannes Schindelin
2017-08-09 22:42         ` Stefan Beller
2017-08-10  9:38           ` Johannes Schindelin
2017-08-10 16:41             ` Junio C Hamano
2017-08-10 17:15               ` Brandon Williams
2017-08-10 17:28                 ` Junio C Hamano
2017-08-10 21:30                   ` Brandon Williams
2017-08-11 20:06                     ` Ben Peart
2017-08-14 15:52               ` Johannes Schindelin
2017-09-28 11:41         ` Johannes Schindelin
2017-09-28 17:16           ` Brandon Williams
2017-08-08 17:45 ` Junio C Hamano
2017-08-08 18:03   ` Brandon Williams
2017-08-08 18:25     ` Junio C Hamano
2017-08-09  7:05       ` Junio C Hamano
2017-08-11 17:49         ` Brandon Williams
2017-08-11 19:00           ` Junio C Hamano
2017-08-09 13:01 ` Jeff King
2017-08-09 14:03   ` Ævar Arnfjörð Bjarmason
2017-08-09 22:53     ` Stefan Beller
2017-08-09 23:11       ` Stefan Beller
2017-08-11 17:52         ` Brandon Williams
2017-08-11 21:18           ` Jeff King
2017-08-12  4:39             ` Junio C Hamano
2017-08-13  4:41               ` Jeff King
2017-08-13 16:14                 ` Ramsay Jones
2017-08-13 16:36                   ` Ramsay Jones
2017-08-13 17:33                   ` Junio C Hamano [this message]
2017-08-13 19:17                     ` Ramsay Jones
2017-08-09 23:19       ` Jeff King
2017-08-15  0:40         ` brian m. carlson
2017-08-15  1:03           ` Jonathan Nieder
2017-08-14 21:30 ` [PATCH v2 0/2] clang-format Brandon Williams
2017-08-14 21:30   ` [PATCH v2 1/2] clang-format: outline the git project's coding style Brandon Williams
2017-08-14 22:02     ` Stefan Beller
2017-08-15 13:56       ` Ben Peart
2017-08-15 17:37         ` Brandon Williams
2017-08-14 22:48     ` Jeff King
2017-08-14 22:51       ` Jeff King
2017-08-14 22:54         ` Brandon Williams
2017-08-14 23:01           ` Jeff King
2017-08-16 12:18             ` Johannes Schindelin
2017-08-15 14:28     ` Ben Peart
2017-08-15 17:34       ` Brandon Williams
2017-08-15 17:41         ` Stefan Beller
2017-08-14 21:30   ` [PATCH v2 2/2] Makefile: add style build rule Brandon Williams
2017-08-14 21:53     ` Stefan Beller
2017-08-14 22:25     ` Junio C Hamano
2017-08-14 22:28       ` Stefan Beller
2017-08-14 22:57         ` Jeff King
2017-08-14 23:29           ` Junio C Hamano
2017-08-14 23:47             ` Junio C Hamano
2017-08-15  0:05               ` Brandon Williams
2017-08-15  1:52               ` Jeff King
2017-08-14 21:32   ` [PATCH v2 0/2] clang-format Brandon Williams
2017-08-14 23:06   ` Jeff King
2017-08-14 23:15     ` Stefan Beller
2017-08-15  1:47       ` Jeff King
2017-08-15  3:03         ` Junio C Hamano
2017-08-15  3:38           ` Jeff King

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqqy3qnwexy.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=ramsay@ramsayjones.plus.com \
    --cc=sbeller@google.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).