Todd Zullinger <tmz@xxxxxxxxx> writes: > Alternately, perhaps having Email::Valid as an optional > dependency is worth reconsidering. If it's truly important > to validation, make it a requirement. If it's not, then > drop it to simplify the code and avoid these sort of issues. Reducing the possible "valid" configurations we support is a very tempting proposition. > If I make the git package require it to ensure consistent > behavior then some folks will -quite rightly- complain that > it should not be a requirement. If I keep it an optional > dependency, then debugging becomes more difficult for the > reasons we've seen in these recent (and not-so-recent) > threads. Very true. > I'd lean toward dropping the dependency entirely and leave > the more basic validation of git-send-email in place. That > may not catch every type of address error, but I would argue > that what we do without Email::Valid is perfectly reasonable > for checking basic email address syntax sanity. Yes, it is very tempting, and given that we have to keep our fallback codepath working for those without Email::Valid ANYWAY, as long as the dependency is merely optional, I very much agree with your argument here. > On a related note, one issue¹ we had reported in Fedora > after making Email::Valid a requirement was that it rejected > messages where the local part was too long, per the relevant > RFC's. But these were generated addresses from GitLab. The > addresses worked in practice. While Email::Valid was > technically correct in rejecting such addresses, it didn't > improve the experience of git send-email users. I am of two minds here. I can sympathize with both positions. - Trying to be strict to what we send out to the world by using Email::Valid that tries to be more RFC kosher matches "be strict in what you send out, be lenient in what you receive" mantra - Rejecting what works in practice and in real world tend to help users. If we require Email::Valid, then sriking the balance between the above two will entirely become the responsibility of them; any end-user who complains "the validation is overly strict" will get "talk to authors of Email::Valid". If we ditch Email::Valid, it will become _our_ responsibility, which means a bit of extra maintenance burden to this project. But perhaps it is worth it? I dunno. Having Email::Valid as an optional dependency does not place us in a position better than either of these two, so from that point of view, too, I like your "we should either make it required or unused, not an optional dependency" very much. Thanks.
