Re: [GSoC PATCH v3 1/1] Unify SMTP auth error handling

[Yuting Zheng <05zyt30@gmail.com>]

On Fri Mar 14, 2025 at 3:58 AM CST, Junio C Hamano wrote:

Thank you for the thorough review. As a newcomer, I really appreciate you
taking time to help me improve.

> "improves clarity ." is (not well formatted and) a bit subjective
> and does not apply to all three changes the patch is making here,
> does it?

I'll reformat the commit message and split the patch into more detailed
parts.

> Hmph, the interpreter may tolerate the new block-eval "eval {}"
> simple statement that lacks terminating ';' but is this an
> improvement?  The original look more kosher from syntactic point of
> view.  It seems to be totally unrelated change from the rest of the
> patch.

I'll revert it to the original state.

> We seem to already have the comment added by this hunk, since
> 4d31a44a (git-send-email: use git credential to obtain password,
> 2013-02-12).  Am I looking at a wrong version of the source (or a
> wrong version of the patch)?
>
> And curiously we do not seem to have this else clause with the
> comment that is getting removed.

You're correct - this was caused by my failure to rebase before
submission.I'll clean up all duplicate comments.

> As I do not see two evals in our copy of git-send-email.perl source,
> it may be moot at this point to comment on this patch, but if we did
> have a eval block each of the if/else arms, moving the control
> structure around and turning "if eval {} else eval {}" into "eval {
> if ... else ...}" may make it cleaner to see what is going on,
> especially if we plan to extend the choices and add elsif to the
> chain later.

I'll use if/else structure which is more extensible.

> Have a SP between "#" and the comment body.

Understood. I'll rigorously adhere to code style guidelines by adding
space after comment markers.

> I'll stop here, as the patch does not seem to be designed to apply
> to our source tree.

This was caused by my local branch being several commits behind upstream.
I've now synchronized and will resubmit properly.