Jari Aalto <jari.aalto@xxxxxxxxx> writes:
> ---
> git-send-email.perl | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 70 insertions(+), 1 deletions(-)
>
>
>
> ================================
> This is REVISION SET 4, reworked
> ================================
Here you should describe differences from v3 (from previous version),
for easier review.
> +# This maildomain*() code is based on ideas in Perl library Test::Reporter
> +# /usr/share/perl5/Test/Reporter/Mail/Util.pm ==> sub _maildomain ()
Nice... although it might be better to use Test::Reporter::Mail::Util
as canonical module name (the package can be installed somewhere else,
depending on operating system / distribution, and if one uses
local::lib for local / per-user installation).
> +sub maildomain_net
> +{
> + my $maildomain;
> +
> + if (eval { require Net::Domain; 1 }) {
> + eval "use Net::Domain";
> + unless ($@) {
> + my $domain = Net::Domain::domainname();
> + $maildomain = $domain
> + unless $^O eq 'darwin' && $domain =~ /\.local$/;
Here should be a comment 'following what Test::Reporter does' or
something like that.
> + }
> + }
> +
> + return $maildomain;
> +}
You still have duplicated 'require' ('use' is 'require' + 'import')
and check if it succeeded. It should read:
+sub maildomain_net {
+ my $maildomain;
+
+ if (eval { require Net::Domain; 1 }) {
+ my $domain = Net::Domain::domainname();
+ $maildomain = $domain
+ unless $^O eq 'darwin' && $domain =~ /\.local$/;
+ }
+
+ return $maildomain;
+}
In the subroutine below you do not duplicate check for require.
Sidenote: alternate soultion would be to write (with one less level of
indent):
+sub maildomain_net {
+ my $maildomain;
+
+ eval { require Net::Domain; };
+ return if $@;
+
+ $maildomain = Net::Domain::domainname();
+ unless $^O eq 'darwin' && $domain =~ /\.local$/;
+
+ return $maildomain;
+}
> +
> +sub maildomain_mta
> +{
Use the same Perl convention that used elsewhere in git-send-email.perl
(this is usually used Perl style).
+sub maildomain_mta {
> + my $maildomain;
> +
> + if (eval { require Net::SMTP; 1 }) {
> + for my $host (qw(mailhost localhost)) {
> + my $smtp = Net::SMTP->new($host);
> + if (defined $smtp) {
> + my $domain = $smtp->domain;
> + $smtp->quit;
> +
> + $maildomain = $domain
> + unless $^O eq 'darwin' && $domain =~ /\.local$/;
> +
> + last if $maildomain;
> + }
> + }
> + }
> +
> + return $maildomain;
> +}
> +
> +sub maildomain
> +{
> + return maildomain_net() || maildomain_mta() || $mail_domain_default;
> +}
Nice.
--
Jakub Narebski
Poland
ShadeHawk on #git
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html