Thread (94 messages) 94 messages, 6 authors, 2024-11-06

Re: [PATCH 3/3] parse: replace atoi() with strtoul_ui() and strtol_i()

From: <hidden>
Date: 2024-10-14 18:36:49

On 14/10/2024 17:26, Usman Akinyemi wrote:
On Mon, Oct 14, 2024 at 4:13 PM Usman Akinyemi
quoted
On Mon, Oct 14, 2024 at 2:55 PM Phillip Wood [off-list ref] 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.
quoted
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

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help