On 11/05/2015 11:06 AM, Lukas Slebodnik wrote:
On (05/11/15 10:52), Sumit Bose wrote:
> On Thu, Nov 05, 2015 at 10:29:41AM +0100, Lukas Slebodnik wrote:
>> On (05/11/15 09:50), Pavel Reichl wrote:
>>>
>>>
>>> On 11/05/2015 08:59 AM, Lukas Slebodnik wrote:
>>>> On (04/11/15 18:58), Pavel Reichl wrote:
>>>>>
>>>>>
>>>>> On 11/04/2015 06:41 PM, Sumit Bose wrote:
>>>>>> On Wed, Nov 04, 2015 at 06:00:34PM +0100, Pavel Reichl wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 11/04/2015 03:02 PM, Lukas Slebodnik wrote:
>>>>>>>> On (04/11/15 14:40), Pavel Reichl wrote:
>>>>>>>
>>>>>>>>>
>>>>>>>>> Also one of few not yet tested corners of pam_reply()
- 'Printing account
>>>>>>>>> expiration warning for sshd' requires pam_service
to be sshd, but I can
>>>>>>>>> change when I will write another test (it might
happen soon as pcech is
>>>>>>>>> working on similar change in adjacent code).
>>>>>>>>>
>>>>>>>> That's exactly the reason why the default value
should not be sshd.
>>>>>>>
>>>>>>> I don't understand why this would be the reason can you
elaborate?
>>>>>>
>>>>>> Because a default value should not lead to a code path which
handles a
>>>>>> special case. The general PAM responder test should not run
through the
>>>>>> 'sshd' case in pam_reply() only if the service is set
explicitly to
>>>>>> 'sshd' this features should be tests.
>>>>>
>>>>> Sure, but just setting sshd would not lead to execution of this
branch AFAIK
>>>>> at least pam verbosity would be needed to be set. As said before
service
>>>>> value doesn't matter in current test so IMO this patch would only
set service
>>>>> to more sane default however I agree with you that 'generic
service name' is
>>>>> not a bad idea at all.
>>>>>
>>>> We spent enormous time with such unimportant patch.
>>>>
>>>> I pushed patch with Sumit's propsal "pam_test_service"
>>>> using one-liner rule.
>>>
>>> So this rule really allows you to:
>>>
>>> 1) amend patch yourself,
>>> 2) change authorship of the patch,
>> Sumit proposed "pam_test_service" so he is author.
>
> Pavel proposed to change the service name first.
>
>>
>>> 3) ack the patch yourself,
>> yes
>>
>>> 4) push the patch yourself,
>> yes
>>
>> But only for oneliners
>
> yes, this rule exists but imo it should be only used to un-break build
There was a disagreement in the content of patch.
Really? I think I expressed an agreement with Sumit's proposal.
On (04/11/15 18:58), Pavel Reichl wrote:
however I agree with you that 'generic service name' is not a
bad idea at all
IMHO "ssh" was good enough and nothing needed to be
changed.
But Pavel insisted on a change but there was disagreement about using "sshd".
I don't think I insisted:
On (04/11/15 13:14), Pavel Reichl wrote:
But If you don't like the change feel free to drop it, it's
no doubt a nitpick.
So the oneliner was used for UNBLOCKING review and cut off
unimportant
discussion. Because we could spent many hours about proper name.
And yet here we are. :-(