From: phillip.wood123@gmail.com
Date: Mon, 14 Oct 2024 18:36:46 GMT
Subject: Re: [PATCH 3/3] parse: replace atoi() with strtoul_ui() and strtol_i()
Message-ID: <84dbe9f1-976d-45f8-a49a-d0f942906686@gmail.com>
In-Reply-To: <CAPSxiM-aptyjesMX1H-P5QJjA-6CUonA01Bo84cq2_t==TqFgw@mail.gmail.com>

On 14/10/2024 17:26, Usman Akinyemi wrote:
> On Mon, Oct 14, 2024 at 4:13 PM Usman Akinyemi
>> On Mon, Oct 14, 2024 at 2:55 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>> I got this from a leftoverbit which the main issue was reported as
>> bug. https://public-inbox.org/git/CAC4O8c-nuOTS=a0sVp1603KaM2bZjs+yNZzdAaa5CGTNGFE7hQ@mail.gmail.com/
>> For the test, I should have the test as another patch right ?

In general you should add tests in the same commit as the code changes 
that they test. In this instance I think you want to split this patch 
into three, one patch for git-daemon, one for imap-send and one for the 
merge marker config changes. Each patch should have a commit message 
explaining the changes and whether they change the behavior of the code 
(for example rejecting non-numbers) and add some tests. Note that I 
don't think it is possible to test the imap-send changes but the other 
two should be easy enough. The tests should be added to one of the 
existing test files that are testing the code being changed.

>> Thanks.
> Also, do I need to add the reference which mentions the leftoverbit in
> the commit message?

I'm not sure that's necessary so long as you explain the reason for the 
changes in the commit message.


Best Wishes

Phillip