Re: [PATCH v3 2/3] t0021: implementation the rot13-filter.pl script in C

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Junio,

On Mon, 1 Aug 2022, Junio C Hamano wrote:

> Matheus Tavares <matheus.bernardino@xxxxxx> writes:
>
> > +		/* Read until flush */
> > +		while ((buf = packet_read_line(0, &size))) {
> > +			if (!strcmp(buf, "can-delay=1")) {
> > +				entry = strmap_get(&delay, pathname);
> > +				if (entry && !entry->requested) {
> > +					entry->requested = 1;
> > +				} else if (!entry && always_delay) {
> > +					add_delay_entry(pathname, 1, 1);
> > +				}
>
> These are unnecessary {} around single statement blocks, but let's
> let it pass in a test helper.

I would like to encourage you to think of ways how this project could
avoid the cost (mental space, reviewer time, back and forth between
contributor and reviewer) of such trivial code formatting issues.

My favored solution would be to adjust the code formatting rules in Git to
such an extent that it can be completely automated, whether via a
`clang-format-diff` rule [*1*] or via an adapted `checkpatch` [*2*] or via
something that is modeled after cURL's `checksrc` script [*3*].

It costs us too much time, and is too annoying all around, having to spend
so many brain cycles on code style (which people like me find much less
interesting than the actual, functional changes).

I'd much rather focus on the implementation of the rot13 filter and
potentially how this patch could give rise to even broader enhancements to
Git's source code that eventually have a user-visible, positive impact.

Ciao,
Dscho

Footnote *1*: https://lore.kernel.org/git/YstJl+5BPyR5RWnR@xxxxxxxxxxxxxxxxxxxxxxxxxxxx/
Footnote *2*: https://lore.kernel.org/git/xmqqbktvl0s4.fsf@gitster.g/
Footnote *3*: https://github.com/curl/curl/blob/master/scripts/checksrc.pl




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux