git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Karthik Nayak <karthik.188@gmail.com>
To: karthik.188@gmail.com
Cc: git@vger.kernel.org, gitster@pobox.com, jltobler@gmail.com,
	toon@iotcl.com
Subject: [PATCH v2 0/3] clang-format: fix rules to make the CI job cleaner
Date: Thu, 10 Oct 2024 19:59:08 +0200	[thread overview]
Message-ID: <cover.1728582927.git.karthik.188@gmail.com> (raw)
In-Reply-To: <CAOLa=ZRvFBhageS65uE5enzLBz7H_CAvvnEcPsi_QAi0exRx2w@mail.gmail.com>

The clang-format CI job is currently cluttered due to too many errors being
reported. See some of the examples here:

* https://gitlab.com/gitlab-org/git/-/jobs/7854601948
* https://gitlab.com/gitlab-org/git/-/jobs/7843131109

So modify the clang-format with the following changes:
1. Modify the column limit to 96 to provide some slack so we can make code more
readable and don't have to be constrained to 80 characters always. This is the
cause of most of the errors reported by the tool and should cleanup the reports
so we can actually focus on the real remaining issues.
2. Don't align expressions after linebreaks to ensure that we instead rely on
'ContinuationIndentWidth'. This fix is rather small and ensures that instead of
trying to align wrapped expressions, we follow the indentation width.
3. Align the macro definitions. This is something we follow to keep the macros
readable. 

I will still keep monitoring the jobs from time to time to ensure we can fine
tune more as needed, if someone see's something odd, do keep me in the loop.

Thanks

Changes over the previous version:
1. Change the column limit from 0 to 96, this way we still have a column limit
but provide some slack.
2. Add an example for the third commit.

Karthik Nayak (3):
  clang-format: change column limit to 96 characters
  clang-format: don't align expressions after linebreaks
  clang-format: align consecutive macro definitions

 .clang-format | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

Range-diff against v1:
1:  34cd0c29f6 ! 1:  e22ffbe0f6 clang-format: don't enforce the column limit
    @@ Metadata
     Author: Karthik Nayak <karthik.188@gmail.com>
     
      ## Commit message ##
    -    clang-format: don't enforce the column limit
    +    clang-format: change column limit to 96 characters
     
         The current value for the column limit is set to 80. While this is as
         expected, we often prefer readability over this strict limit. This means
         it is common to find code which extends over 80 characters. So let's
    -    change the column limit to be 0 instead. This ensures that the formatter
    -    doesn't complain about code strictly not following the column limit.
    +    change the column limit to be 96 instead. This provides some slack so we
    +    can ensure readability takes preference over the 80 character hard
    +    limit.
     
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
    @@ .clang-format: UseTab: Always
      ContinuationIndentWidth: 8
     -ColumnLimit: 80
     +
    -+# While we recommend keeping column limit to 80, we don't want to
    -+# enforce it as we generally are more lenient with this rule and
    -+# prefer to prioritize readability.
    -+ColumnLimit: 0
    ++# While we recommend keeping column limit to 80, we want to also provide
    ++# some slack to maintain readability.
    ++ColumnLimit: 96
      
      # C Language specifics
      Language: Cpp
2:  3557012fea = 2:  b55d5d2c14 clang-format: don't align expressions after linebreaks
3:  ad023a6cf6 ! 3:  6ebcd2690e clang-format: align consecutive macro definitions
    @@ Metadata
      ## Commit message ##
         clang-format: align consecutive macro definitions
     
    -    We generally align consecutive macro definitions for better readability.
    +    We generally align consecutive macro definitions for better readability:
    +
    +      #define OUTPUT_ANNOTATE_COMPAT      (1U<<0)
    +      #define OUTPUT_LONG_OBJECT_NAME     (1U<<1)
    +      #define OUTPUT_RAW_TIMESTAMP        (1U<<2)
    +      #define OUTPUT_PORCELAIN            (1U<<3)
    +
    +    over
    +
    +      #define OUTPUT_ANNOTATE_COMPAT (1U<<0)
    +      #define OUTPUT_LONG_OBJECT_NAME (1U<<1)
    +      #define OUTPUT_RAW_TIMESTAMP (1U<<2)
    +      #define OUTPUT_PORCELAIN (1U<<3)
    +
         So let's add the rule in clang-format to follow this.
     
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
-- 
2.47.0


  parent reply	other threads:[~2024-10-10 17:59 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-09 12:51 [PATCH 0/3] clang-format: fix rules to make the CI job cleaner Karthik Nayak
2024-10-09 12:55 ` [PATCH 1/3] clang-format: don't enforce the column limit Karthik Nayak
2024-10-09 15:45   ` Justin Tobler
2024-10-09 22:32     ` Junio C Hamano
2024-10-10 16:48       ` karthik nayak
2024-10-09 12:56 ` [PATCH 2/3] clang-format: don't align expressions after linebreaks Karthik Nayak
2024-10-09 12:56 ` [PATCH 3/3] clang-format: align consecutive macro definitions Karthik Nayak
2024-10-10  8:27   ` Toon Claes
2024-10-10 17:59 ` Karthik Nayak [this message]
2024-10-10 17:59   ` [PATCH v2 1/3] clang-format: change column limit to 96 characters Karthik Nayak
2024-10-10 18:11     ` Kyle Lippincott
2024-10-10 19:49       ` karthik nayak
2024-10-10 20:09         ` Kyle Lippincott
2024-10-10 17:59   ` [PATCH v2 2/3] clang-format: don't align expressions after linebreaks Karthik Nayak
2024-10-10 17:59   ` [PATCH v2 3/3] clang-format: align consecutive macro definitions Karthik Nayak
2024-10-12  1:49 ` [PATCH v3 0/3] clang-format: fix rules to make the CI job cleaner Karthik Nayak
2024-10-12  1:49   ` [PATCH v3 1/3] clang-format: re-adjust line break penalties Karthik Nayak
2024-10-14  9:08     ` Toon Claes
2024-10-14 21:14       ` Taylor Blau
2024-10-15 11:35         ` karthik nayak
2024-10-15 11:20       ` karthik nayak
2024-10-14 20:59     ` Kyle Lippincott
2024-10-15 12:37       ` karthik nayak
2024-10-16  1:38         ` Kyle Lippincott
2024-10-16 21:17           ` karthik nayak
2024-10-12  1:49   ` [PATCH v3 2/3] clang-format: align consecutive macro definitions Karthik Nayak
2024-10-14 21:12     ` Kyle Lippincott
2024-10-15  7:57       ` karthik nayak
2024-10-12  1:49   ` [PATCH v3 3/3] clang-format: don't align expressions after linebreaks Karthik Nayak
2024-10-14 21:23     ` Kyle Lippincott
2024-10-15 11:17       ` karthik nayak
2024-10-18  8:46 ` [PATCH v4 0/2] Subject: clang-format: fix rules to make the CI job cleaner Karthik Nayak
2024-10-18  8:46   ` [PATCH v4 1/2] clang-format: re-adjust line break penalties Karthik Nayak
2024-10-25  2:37     ` Justin Tobler
2024-10-25  9:48       ` karthik nayak
2024-10-18  8:46   ` [PATCH v4 2/2] clang-format: align consecutive macro definitions Karthik Nayak
2024-10-18 21:37   ` [PATCH v4 0/2] Subject: clang-format: fix rules to make the CI job cleaner Taylor Blau
2024-10-20 11:17     ` karthik nayak
2024-10-21 21:48       ` Taylor Blau
2024-10-22  8:31         ` karthik nayak
2024-10-22 16:42           ` Taylor Blau

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=cover.1728582927.git.karthik.188@gmail.com \
    --to=karthik.188@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jltobler@gmail.com \
    --cc=toon@iotcl.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;
as well as URLs for NNTP newsgroup(s).