On Thu, Aug 3, 2017 at 9:11 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: >> diff --git a/perl/Git/Packet.pm b/perl/Git/Packet.pm >> new file mode 100644 >> index 0000000000..aaffecbe2a >> --- /dev/null >> +++ b/perl/Git/Packet.pm >> @@ -0,0 +1,71 @@ >> +package Git::Packet; >> +use 5.008; >> +use strict; >> +use warnings; >> +BEGIN { >> + require Exporter; >> + if ($] < 5.008003) { >> + *import = \&Exporter::import; >> + } else { >> + # Exporter 5.57 which supports this invocation was >> + # released with perl 5.8.3 >> + Exporter->import('import'); >> + } >> +} > > This is merely me being curious, but do we want this boilerplate, > which we do not use in perl/Git.pm but we do in perl/Git/I18N.pm? I don't know. I copied it as I thought that we wanted to support Perl versions starting from 5.8.0, but I am ok to remove it or to leave it depending on what the Perl experts think (CCing AEvar) and what we decide. >> +our @EXPORT = qw( >> + packet_bin_read >> + packet_txt_read >> + packet_bin_write >> + packet_txt_write >> + packet_flush >> + ); >> +our @EXPORT_OK = @EXPORT; > > We can see that you made sure that the only thing 05/40 needs to do > is to use this package and remove the definition of these subs, > without having to touch any caller by first updating the original > implementation in 03/40 and then exporting these names in 04/40. > Knowing that the preparation is nicely done already, it is a bit > irritating to see that 05/40 is a separate patch, as we need to > switch between the patches to see if there is any difference between > the original implementation of the subs, and the replacement > implemented in here. It would have been nicer to have changes in > 04/40 and 05/40 in a single patch. Ok, I have squashed 04/40 and 05/40 together in my current version of this series.