[v4,0/2] Subject: clang-format: fix rules to make the CI job cleaner

Message

Karthik Nayak Oct. 18, 2024, 8:46 a.m. UTC

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 penalties for linebreaks to be more considerate towards
readability. The commit goes into detail explaining how/why.  
2. 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. I made the example in the first commit message a bit clearer so it is easier
to understand.
2. Removed the third commit, since I was convinced that it is good as-is for now. 

Karthik Nayak (2):
  clang-format: re-adjust line break penalties
  clang-format: align consecutive macro definitions

 .clang-format | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

Range-diff against v3:
1:  74bbd2f9db ! 1:  a8c8df5d95 clang-format: re-adjust line break penalties
    @@ Commit message
         to instead have something more readable like:
     
            static const struct strbuf *a_really_really_large_function_name(struct strbuf resolved,
    -              const char *path, int flags)
    +                                                                       const char *path,
    +                                                                       int flags)
    +
    +    (note: the tabs here have been replaced by spaces for easier reading)
     
         This is done by bumping the values of 'PenaltyReturnTypeOnItsOwnLine'
         and 'PenaltyBreakOpenParenthesis' to 300. This is so that we can allow a
2:  1586d53769 = 2:  fcf965ac74 clang-format: align consecutive macro definitions
3:  36a53299c1 < -:  ---------- clang-format: don't align expressions after linebreaks

Comments

Taylor Blau Oct. 18, 2024, 9:37 p.m. UTC | #1

On Fri, Oct 18, 2024 at 10:46:44AM +0200, Karthik Nayak wrote:
> Karthik Nayak (2):
>   clang-format: re-adjust line break penalties
>   clang-format: align consecutive macro definitions
>
>  .clang-format | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)

Thanks, will queue. Are we ready to start merging this one down?

Thanks,
Taylor

Karthik Nayak Oct. 20, 2024, 11:17 a.m. UTC | #2

Taylor Blau <[e-mail]> writes:

> On Fri, Oct 18, 2024 at 10:46:44AM +0200, Karthik Nayak wrote:
>> Karthik Nayak (2):
>>   clang-format: re-adjust line break penalties
>>   clang-format: align consecutive macro definitions
>>
>>  .clang-format | 16 ++++++++++------
>>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> Thanks, will queue. Are we ready to start merging this one down?
>

I'd wait for some reviews :)

> Thanks,
> Taylor

Taylor Blau Oct. 21, 2024, 9:48 p.m. UTC | #3

On Sun, Oct 20, 2024 at 06:17:58AM -0500, karthik nayak wrote:
> Taylor Blau <[e-mail]> writes:
>
> > On Fri, Oct 18, 2024 at 10:46:44AM +0200, Karthik Nayak wrote:
> >> Karthik Nayak (2):
> >>   clang-format: re-adjust line break penalties
> >>   clang-format: align consecutive macro definitions
> >>
> >>  .clang-format | 16 ++++++++++------
> >>  1 file changed, 10 insertions(+), 6 deletions(-)
> >
> > Thanks, will queue. Are we ready to start merging this one down?
>
> I'd wait for some reviews :)

OK. My impression was that the dust had more or less settled from
earlier rounds. But let's wait.

Thanks,
Taylor

Karthik Nayak Oct. 22, 2024, 8:31 a.m. UTC | #4

Taylor Blau <[e-mail]> writes:

> On Sun, Oct 20, 2024 at 06:17:58AM -0500, karthik nayak wrote:
>> Taylor Blau <[e-mail]> writes:
>>
>> > On Fri, Oct 18, 2024 at 10:46:44AM +0200, Karthik Nayak wrote:
>> >> Karthik Nayak (2):
>> >>   clang-format: re-adjust line break penalties
>> >>   clang-format: align consecutive macro definitions
>> >>
>> >>  .clang-format | 16 ++++++++++------
>> >>  1 file changed, 10 insertions(+), 6 deletions(-)
>> >
>> > Thanks, will queue. Are we ready to start merging this one down?
>>
>> I'd wait for some reviews :)
>
> OK. My impression was that the dust had more or less settled from
> earlier rounds. But let's wait.
>
> Thanks,
> Taylor

I'd be happy if it merged down, I'll see if someone from GitLab can help
with a review.

Thanks,
Karthik

Taylor Blau Oct. 22, 2024, 4:42 p.m. UTC | #5

On Tue, Oct 22, 2024 at 04:31:37AM -0400, karthik nayak wrote:
> Taylor Blau <[e-mail]> writes:
>
> > On Sun, Oct 20, 2024 at 06:17:58AM -0500, karthik nayak wrote:
> >> Taylor Blau <[e-mail]> writes:
> >>
> >> > On Fri, Oct 18, 2024 at 10:46:44AM +0200, Karthik Nayak wrote:
> >> >> Karthik Nayak (2):
> >> >>   clang-format: re-adjust line break penalties
> >> >>   clang-format: align consecutive macro definitions
> >> >>
> >> >>  .clang-format | 16 ++++++++++------
> >> >>  1 file changed, 10 insertions(+), 6 deletions(-)
> >> >
> >> > Thanks, will queue. Are we ready to start merging this one down?
> >>
> >> I'd wait for some reviews :)
> >
> > OK. My impression was that the dust had more or less settled from
> > earlier rounds. But let's wait.
> >
> > Thanks,
> > Taylor
>
> I'd be happy if it merged down, I'll see if someone from GitLab can help
> with a review.

Having additional reviewer eyes is much appreciated. Let's err on the
side of that rather than rushing a topic if you don't feel that there is
consensus yet.

Thanks,
Taylor