Re: [GSoC] acknowledging mistakes

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 1, 2019, 02:51 UTC
Message-ID: <xmqqef7r9uil.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20190228190242.20680-1-rohit.ashiwal265@gmail.com>

Rohit Ashiwal <rohit.ashiwal265@gmail.com> writes:

> 1. Commit message was less than 50 chars which should be around 72 chars
>    according to coding guide lines. Should I change this to match 72?

Simple things do not need that many letters to tell ;-) The suggestion of 72 is about the maximum.

If you are doing something in a single patch that needs a longer title, it generally is a sign that you are trying to do too much in a single patch and should be splitting the patch into more digestable smaller steps. And the purpose of having a maximum is to nudge patch authors to realize that.

Show 5 quoted lines

> 2. My changes had some uneven use of tabs and spaces, which I made
>    considering that pre-existing code had them too. Is there a
>    possibility to change the whole code according to CodingGuidelines?
>    If yes should I only change my code according to guidelines or the
>    whole file?

I think you are talking about t3600, which uses an ancient style. If this were a real project, then the preferred order would be

 - A preliminary patch (or a series of patches) that modernizes
   existing tests in t3600.  Just style updates and adding or
   removing nothing else.

 - Update test that use "test -f" and friends to use the helpers in
   t3600.

> 3. There is no helper function for `test -s` but Rafael suggested we can
>    make use of other helper functions to provide similar functionality,
>    if we can.

If we often see if a path is an non-empty file in our tests (not limited to t3600), then it may make sense to add a new helper test_path_is_non_empty_file in t/test-lib-functions.sh next to where test_path_is_file and friends are defined.

Thanks.

[jch: I am still mostly offline til the next week, but I had a chance to sit in front of my mailbox long enough, so...]