Subject: Re: bug#38284: 27.0.50; [PATCH] Make auth-source-pass-search understand port lists

From: João Távora <[email-masked]>
Date: Fri, 22 Nov 2019 09:35:38 +0000

Damien Cassou <[email-masked]> writes:

> João Távora <[email-masked]> writes:
>>> can you please rename "d" to "domain"?
>>
>> ok.  I do call your attention that it was already the
>> single letter n there
>
> yes I saw. This should never have been merged like that. Thank you for
> improving the code.

I think the single letter idiom is fine there.  It was just the wrong
letter.  By the way, I've left the p for the port, because calling it
"port", while it would work, would seriously confuse a reader.

>>> Can you please add a unit test covering this new use-case?
>>
>> No. This is too much work for such a trivial change
>
> I care a lot about the automated testing of the code I write.

Certainly, I care a lot, too.  I don't write tests for these changes out
of principle, not out of lazyness.  Most, if not all, the projects I
manage have automated tests.

> I won't try to convince you though. Can you please merge the attached
> patch with yours?

No, but you can do that, because it's your work (I can push it for you
though).  Anyway, now I read the test you wrote, I agree it's a good
test.  You are testing auth-source-pass-match-entry-p, much higher up
than auth-source-pass--generate-entry-suffixes, the function I changed.

Of course, only someone who was involved in the design would be able to
confidently place the tests at that correct level, the finding of which
is the most difficult part.

My advice and personal opinion is to later use this and more such tests
to perhaps redesign/cleanup the auth-source-pass.el library, which seems
needlessly complicated in the little stuff like the function I touched.
(to be fair it wasn't much helped by the style of the auth-source.el
parent library.)

João