Re: [PATCH v5 04/40] Add Git/Packet.pm from parts of t0021/rot13-filter.pl

Christian Couder [off-list ref] writes:

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?

+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.

On Thu, Aug 3, 2017 at 9:11 PM, Junio C Hamano [off-list ref] wrote:

quoted

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.

quoted

+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.