Re: [patch] nss: fix nss_database_lookup2's alternate handling [27416]

[Carlos O'Donell <carlos@redhat.com>]

On 2/22/21 8:16 PM, DJ Delorie wrote:
> "Carlos O'Donell" <carlos@redhat.com> writes:
>> On 2/19/21 5:22 PM, DJ Delorie via Libc-alpha wrote:
>>>
>>> https://sourceware.org/bugzilla/show_bug.cgi?id=27416
>>
>> Could you please provide the full commit message you plan to
>> use for this commit?
> 
> That was it.  Is more required?

The commit message should allow a reviewer to understand the
problem in the briefest form without external reference. A few
sentences about how we got here and why, and what corrective
steps are being taken would be useful for a commit message.

For inspiration please see:
https://sourceware.org/glibc/wiki/Contribution%20checklist#Commit_message_body

A full URL reference to sourceware is also not required.

The subject bug reference should be in one of two forms for
automated processing: (bug xxxx), or [BZ #xxxx]. Please adjust

>> If you commit your change locally then do:
>> git format-patch HEAD~1
>> git send-email mydiff.patch
> 
> I commit my change locally, use gitk to extract it, then mail it on a
> different machine.

That's all OK, you can run git send-email on that remote system if you like
(avoids MUA problems).

https://sourceware.org/glibc/wiki/Contribution%20checklist#Generate_patch

`git format-patch` is the recommended way to get the format that a reviewer 
is expecting.

>> Please repost v2.
> 
> What changes are required?

Please repost v2 with the suggested commit message changes for review.

Reviewing the commit message is part of the review.

When the review is complete the only difference between what is reviewed
and what is committed should be the added Reviewed-by line.
 
>>> +  else if (strcmp (p->pw_name, "name100") != 0)
>>> +    FAIL_EXIT1("getpwuid-compat-test1 name100");
>>
>> OK. Password uses compat which redirects to test1 instead of files.
>> So seeing name100 means files was used and that's wrong.
> 
> "name100" comes from test1; we're testing that it *is* returned, which
> means test1 *is* being used to back compat.

OK.

-- 
Cheers,
Carlos.