Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PATCH] Super Quick Patch Guide Improvements #13225

Closed
p5pRT opened this issue Sep 4, 2013 · 38 comments
Closed

[PATCH] Super Quick Patch Guide Improvements #13225

p5pRT opened this issue Sep 4, 2013 · 38 comments

Comments

@p5pRT
Copy link

p5pRT commented Sep 4, 2013

Migrated from rt.perl.org#119599 (status was 'resolved')

Searchable as RT119599$

@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2013

From @Smylers

This is a bug report for perl from smylers@​stripey.com,
generated with the help of perlbug 1.39 running under perl 5.19.4.


I'm suggesting a few tweaks to the Super Quick Patch Guide, based on my
experiences of using it to submit patches.

There is the risk of the issues MJD discusses here, that adding more
text to the guide makes it less-quick and overall less useful​:
http​://blog.plover.com/prog/featurism.html

My suggested additions are all things that would've helped me (and saved
the time of Porters I then had to ask for assistance) had they been in
the Guide.



Flags​:
  category=docs
  severity=low


Site configuration information for perl 5.19.4​:

Configured by smylers at Tue Sep 3 10​:58​:31 BST 2013.

Summary of my perl5 (revision 5 version 19 subversion 4) configuration​:
  Commit id​: 5565c73
  Platform​:
  osname=linux, osvers=3.8.0-30-generic, archname=x86_64-linux
  uname='linux fozzie 3.8.0-30-generic #44-ubuntu smp thu aug 22 20​:52​:24 utc 2013 x86_64 x86_64 x86_64 gnulinux '
  config_args='-des -Dusedevel'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=undef, usemultiplicity=undef
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=define, use64bitall=define, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O2',
  cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.7.3', gccosandvers=''
  intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
  ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib
  libs=-lnsl -ldl -lm -lcrypt -lutil -lc
  perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
  libc=, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.17'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector'


@​INC for perl 5.19.4​:
  lib
  /home/smylers/lib/perl5/site_perl
  /home/smylers/lib/perl5
  /usr/local/lib/perl5/site_perl/5.19.4/x86_64-linux
  /usr/local/lib/perl5/site_perl/5.19.4
  /usr/local/lib/perl5/5.19.4/x86_64-linux
  /usr/local/lib/perl5/5.19.4
  .


Environment for perl 5.19.4​:
  HOME=/home/smylers
  LANG=en_GB.utf8
  LANGUAGE=en_GB​:en
  LC_COLLATE=C
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=/home/smylers/bin​:/usr/local/sbin​:/usr/local/bin​:/sbin​:/bin​:/usr/sbin​:/usr/bin​:/usr/X11R6/bin​:/usr/games
  PERL5LIB=/home/smylers/lib/perl5/site_perl​:/home/smylers/lib/perl5
  PERL_BADLANG (unset)
  PERL_CPANM_OPT=--sudo --prompt
  SHELL=/bin/bash

Smylers
--
Stop drug companies hiding negative research results.
Sign the AllTrials petition to get all clinical research results published.
Read more​: http​://www.alltrials.net/blog/the-alltrials-campaign/

@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2013

From @Smylers

0001-perlhack.pod-tidied.patch
From cb3f695e419e30c1b7af57ff29d1d0985a81c1e9 Mon Sep 17 00:00:00 2001
From: Smylers <Smylers@stripey.com>
Date: Wed, 4 Sep 2013 10:31:25 +0100
Subject: [PATCH 1/8] perlhack.pod tidied
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------1.8.1.2"

This is a multi-part message in MIME format.
--------------1.8.1.2
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


In accordance with the comment at the top of the file, before I make other
changes to it.
---
 pod/perlhack.pod | 87 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 44 insertions(+), 43 deletions(-)


--------------1.8.1.2
Content-Type: text/x-patch; name="0001-perlhack.pod-tidied.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0001-perlhack.pod-tidied.patch"

diff --git a/pod/perlhack.pod b/pod/perlhack.pod
index 6403bb1..646606f 100644
--- a/pod/perlhack.pod
+++ b/pod/perlhack.pod
@@ -119,8 +119,8 @@ Perl core.
 =head1 GETTING THE PERL SOURCE
 
 All of Perl's source code is kept centrally in a Git repository at
-I<perl5.git.perl.org>.  The repository contains many Perl revisions from
-Perl 1 onwards and all the revisions from Perforce, the previous
+I<perl5.git.perl.org>.  The repository contains many Perl revisions
+from Perl 1 onwards and all the revisions from Perforce, the previous
 version control system.
 
 For much more detail on using git with the Perl repository, please see
@@ -206,9 +206,9 @@ in a month.  Please remember that the Perl 5 developers are all
 volunteers, and be polite.
 
 Changes are always applied directly to the main development branch,
-called "blead".  Some patches may be backported to a maintenance branch.
-If you think your patch is appropriate for the maintenance branch,
-please explain why when you submit it.
+called "blead".  Some patches may be backported to a maintenance
+branch. If you think your patch is appropriate for the maintenance
+branch, please explain why when you submit it.
 
 =head2 Getting your patch accepted
 
@@ -220,8 +220,8 @@ can do to help the Perl 5 Porters accept your patch.
 If you used git to check out the Perl source, then using C<git
 format-patch> will produce a patch in a style suitable for Perl.  The
 C<format-patch> command produces one patch file for each commit you
-made.  If you prefer to send a single patch for all commits, you can use
-C<git diff>.
+made.  If you prefer to send a single patch for all commits, you can
+use C<git diff>.
 
   % git checkout blead
   % git pull
@@ -305,12 +305,12 @@ readers understand what you did and why you did it.
 
 =head3 Comments, Comments, Comments
 
-Be sure to adequately comment your code.  While commenting every line is
-unnecessary, anything that takes advantage of side effects of
+Be sure to adequately comment your code.  While commenting every line
+is unnecessary, anything that takes advantage of side effects of
 operators, that creates changes that will be felt outside of the
 function being patched, or that others may find confusing should be
-documented.  If you are going to err, it is better to err on the side of
-adding too many comments than too few.
+documented.  If you are going to err, it is better to err on the side
+of adding too many comments than too few.
 
 The best comments explain I<why> the code does what it does, not I<what
 it does>.
@@ -393,8 +393,8 @@ In-line functions that are in headers that are accessible to XS code
 need to be able to compile without warnings with commonly used extra
 compilation flags, such as gcc's C<-Wswitch-default> which warns
 whenever a switch statement does not have a "default" case.  The use of
-these extra flags is to catch potential problems in legal C code, and is
-often used by Perl aggregators, such as Linux distributors.
+these extra flags is to catch potential problems in legal C code, and
+is often used by Perl aggregators, such as Linux distributors.
 
 =back
 
@@ -508,8 +508,8 @@ core.
 
 For changes significant enough to warrant a F<pod/perldelta.pod> entry,
 the porters will greatly appreciate it if you submit a delta entry
-along with your actual change.  Significant changes include, but are not
-limited to:
+along with your actual change.  Significant changes include, but are
+not limited to:
 
 =over 4
 
@@ -692,9 +692,9 @@ authors, ... Perl is supposed to be easy.
 Working code is always preferred to pie-in-the-sky ideas.  A patch to
 add a feature stands a much higher chance of making it to the language
 than does a random feature request, no matter how fervently argued the
-request might be.  This ties into "Will it be useful?", as the fact that
-someone took the time to make the patch demonstrates a strong desire
-for the feature.
+request might be.  This ties into "Will it be useful?", as the fact
+that someone took the time to make the patch demonstrates a strong
+desire for the feature.
 
 =head1 TESTING
 
@@ -703,10 +703,11 @@ The core uses the same testing style as the rest of Perl, a simple
 considerations.
 
 There are three ways to write a test in the core: L<Test::More>,
-F<t/test.pl> and ad hoc C<print $test ? "ok 42\n" : "not ok 42\n">.  The
-decision of which to use depends on what part of the test suite you're
-working on.  This is a measure to prevent a high-level failure (such as
-Config.pm breaking) from causing basic functionality tests to fail.
+F<t/test.pl> and ad hoc C<print $test ? "ok 42\n" : "not ok 42\n">. 
+The decision of which to use depends on what part of the test suite
+you're working on.  This is a measure to prevent a high-level failure
+(such as Config.pm breaking) from causing basic functionality tests to
+fail.
 
 The F<t/test.pl> library provides some of the features of
 L<Test::More>, but avoids loading most modules and uses as few core
@@ -721,9 +722,9 @@ Protocol|http://testanything.org>.
 
 Since we don't know if require works, or even subroutines, use ad hoc
 tests for these three.  Step carefully to avoid using the feature being
-tested.  Tests in F<t/opbasic>, for instance, have been placed there rather
-than in F<t/op> because they test functionality which F<t/test.pl> presumes
-has already been demonstrated to work.
+tested.  Tests in F<t/opbasic>, for instance, have been placed there
+rather than in F<t/op> because they test functionality which
+F<t/test.pl> presumes has already been demonstrated to work.
 
 =item * F<t/cmd>, F<t/run>, F<t/io> and F<t/op>
 
@@ -741,10 +742,10 @@ used.  You can also use the full suite of core modules in the tests.
 =back
 
 When you say "make test", Perl uses the F<t/TEST> program to run the
-test suite (except under Win32 where it uses F<t/harness> instead).  All
-tests are run from the F<t/> directory, B<not> the directory which
-contains the test.  This causes some problems with the tests in F<lib/>,
-so here's some opportunity for some patching.
+test suite (except under Win32 where it uses F<t/harness> instead). 
+All tests are run from the F<t/> directory, B<not> the directory which
+contains the test.  This causes some problems with the tests in
+F<lib/>, so here's some opportunity for some patching.
 
 You must be triply conscious of cross-platform concerns.  This usually
 boils down to using L<File::Spec> and avoiding things like C<fork()>
@@ -782,8 +783,8 @@ Run the test suite with the F<t/harness> controlling program, instead
 of F<t/TEST>.  F<t/harness> is more sophisticated, and uses the
 L<Test::Harness> module, thus using this test target supposes that perl
 mostly works.  The main advantage for our purposes is that it prints a
-detailed summary of failed tests at the end.  Also, unlike F<t/TEST>, it
-doesn't redirect stderr to stdout.
+detailed summary of failed tests at the end.  Also, unlike F<t/TEST>,
+it doesn't redirect stderr to stdout.
 
 Note that under Win32 F<t/harness> is always used instead of F<t/TEST>,
 so there is no special "test_harness" target.
@@ -816,8 +817,8 @@ non-conflicting test scripts itself, and there is no standard interface
 to C<make> utilities to interact with their job schedulers.
 
 Note that currently some test scripts may fail when run in parallel
-(most notably F<ext/IO/t/io_dir.t>).  If necessary, run just the failing
-scripts again sequentially and see if the failures go away.
+(most notably F<ext/IO/t/io_dir.t>).  If necessary, run just the
+failing scripts again sequentially and see if the failures go away.
 
 =head2 Running tests by hand
 
@@ -858,9 +859,9 @@ Run the torture tests as well as the normal set.
 
 =item * -re=PATTERN
 
-Filter the file list so that all the test files run match PATTERN.  Note
-that this form is distinct from the B<-re LIST OF PATTERNS> form below
-in that it allows the file list to be provided as well.
+Filter the file list so that all the test files run match PATTERN. 
+Note that this form is distinct from the B<-re LIST OF PATTERNS> form
+below in that it allows the file list to be provided as well.
 
 =item * -re LIST OF PATTERNS
 
@@ -898,9 +899,9 @@ F<./perl>).
 
 =item * PERL_SKIP_TTY_TEST
 
-if set, tells to skip the tests that need a terminal.  It's actually set
-automatically by the Makefile, but can also be forced artificially by
-running 'make test_notty'.
+if set, tells to skip the tests that need a terminal.  It's actually
+set automatically by the Makefile, but can also be forced artificially
+by running 'make test_notty'.
 
 =back
 
@@ -925,8 +926,8 @@ This sets a variable in op/numconvert.t.
 =item * PERL_TEST_MEMORY
 
 Setting this variable includes the tests in F<t/bigmem/>.  This should
-be set to the number of gigabytes of memory available for testing,
-eg. C<PERL_TEST_MEMORY=4> indicates that tests that require 4GiB of
+be set to the number of gigabytes of memory available for testing, eg.
+C<PERL_TEST_MEMORY=4> indicates that tests that require 4GiB of
 available memory can be run safely.
 
 =back
@@ -1040,8 +1041,8 @@ README if you find anything missing or changed over a new OS release.
 =item *
 
 Find an area of Perl that seems interesting to you, and see if you can
-work out how it works.  Scan through the source, and step over it in the
-debugger.  Play, poke, investigate, fiddle! You'll probably get to
+work out how it works.  Scan through the source, and step over it in
+the debugger.  Play, poke, investigate, fiddle! You'll probably get to
 understand not just your chosen area but a much wider range of
 F<perl>'s activity as well, and probably sooner than you'd think.
 

--------------1.8.1.2--


@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2013

From @Smylers

0002-Consistent-indent-on-shell-commands.patch
From 2860957b25b7fbe430b1beb0031f5791a7cae729 Mon Sep 17 00:00:00 2001
From: Smylers <Smylers@stripey.com>
Date: Wed, 4 Sep 2013 11:32:24 +0100
Subject: [PATCH 2/8] Consistent indent on shell commands
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------1.8.1.2"

This is a multi-part message in MIME format.
--------------1.8.1.2
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


Most verbatim lines with shell commands had 2 spaces before the % prompt.
A few had 1 or 4 spaces. Make them all 2.

diff -b shows no changes for this commit.
---
 pod/perlhack.pod | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)


--------------1.8.1.2
Content-Type: text/x-patch; name="0002-Consistent-indent-on-shell-commands.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0002-Consistent-indent-on-shell-commands.patch"

diff --git a/pod/perlhack.pod b/pod/perlhack.pod
index 646606f..1862d02 100644
--- a/pod/perlhack.pod
+++ b/pod/perlhack.pod
@@ -60,8 +60,8 @@ Assuming your patch consists of a single git commit, the following
 writes the file as a MIME attachment, and sends it with a meaningful
 subject:
 
- % git format-patch -1 --attach
- % perlbug -s "[PATCH] $(git log -1 --oneline HEAD)" -f 0001-*.patch
+  % git format-patch -1 --attach
+  % perlbug -s "[PATCH] $(git log -1 --oneline HEAD)" -f 0001-*.patch
 
 The perlbug program will ask you a few questions about your email
 address and the patch you're submitting.  Once you've answered them it
@@ -154,18 +154,18 @@ found at L<http://github.com/mirrors/perl>.
 You can also choose to use rsync to get a copy of the current source
 tree for the bleadperl branch and all maintenance branches:
 
-    % rsync -avz rsync://perl5.git.perl.org/perl-current .
-    % rsync -avz rsync://perl5.git.perl.org/perl-5.12.x .
-    % rsync -avz rsync://perl5.git.perl.org/perl-5.10.x .
-    % rsync -avz rsync://perl5.git.perl.org/perl-5.8.x .
-    % rsync -avz rsync://perl5.git.perl.org/perl-5.6.x .
-    % rsync -avz rsync://perl5.git.perl.org/perl-5.005xx .
+  % rsync -avz rsync://perl5.git.perl.org/perl-current .
+  % rsync -avz rsync://perl5.git.perl.org/perl-5.12.x .
+  % rsync -avz rsync://perl5.git.perl.org/perl-5.10.x .
+  % rsync -avz rsync://perl5.git.perl.org/perl-5.8.x .
+  % rsync -avz rsync://perl5.git.perl.org/perl-5.6.x .
+  % rsync -avz rsync://perl5.git.perl.org/perl-5.005xx .
 
 (Add the C<--delete> option to remove leftover files.)
 
 To get a full list of the available sync points:
 
-    % rsync perl5.git.perl.org::
+  % rsync perl5.git.perl.org::
 
 =head2 Write access via git
 

--------------1.8.1.2--


@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2013

From @Smylers

0003-Have-perlbug-report-version-being-patched.patch
From 27cd31ebfe338c25deffd0e3872390a749b33ebd Mon Sep 17 00:00:00 2001
From: Smylers <Smylers@stripey.com>
Date: Wed, 4 Sep 2013 10:49:41 +0100
Subject: [PATCH 3/8] Have perlbug report version being patched
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------1.8.1.2"

This is a multi-part message in MIME format.
--------------1.8.1.2
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


In the Super Quick Patch Guide, run the perlbug and perl from the working
copy that the patch is against, so the bug report contains relevant
version and configuration data, rather than that of whichever system perl
the reporter happens to have installed.
---
 pod/perlhack.pod | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


--------------1.8.1.2
Content-Type: text/x-patch; name="0003-Have-perlbug-report-version-being-patched.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0003-Have-perlbug-report-version-being-patched.patch"

diff --git a/pod/perlhack.pod b/pod/perlhack.pod
index 1862d02..d559261 100644
--- a/pod/perlhack.pod
+++ b/pod/perlhack.pod
@@ -61,7 +61,7 @@ writes the file as a MIME attachment, and sends it with a meaningful
 subject:
 
   % git format-patch -1 --attach
-  % perlbug -s "[PATCH] $(git log -1 --oneline HEAD)" -f 0001-*.patch
+  % ./perl -Ilib utils/perlbug -s "[PATCH] $(git log -1 --oneline HEAD)" -f 0001-*.patch
 
 The perlbug program will ask you a few questions about your email
 address and the patch you're submitting.  Once you've answered them it

--------------1.8.1.2--


@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2013

From @Smylers

0004-perlbug-command-wrapped-to-fit-in-79-columns.patch
From 68f21b328749cc436256fd86d96c138d09cc7638 Mon Sep 17 00:00:00 2001
From: Smylers <Smylers@stripey.com>
Date: Wed, 4 Sep 2013 10:54:15 +0100
Subject: [PATCH 4/8] perlbug command wrapped to fit in 79 columns
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------1.8.1.2"

This is a multi-part message in MIME format.
--------------1.8.1.2
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


To pass t/porting/podcheck.t --pedantic

The line-break is inside a $(...), so the lines can be copied-and-pasted
as they are, complete with line-break and extra spaces, and still give the
same output.
---
 pod/perlhack.pod | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


--------------1.8.1.2
Content-Type: text/x-patch; name="0004-perlbug-command-wrapped-to-fit-in-79-columns.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0004-perlbug-command-wrapped-to-fit-in-79-columns.patch"

diff --git a/pod/perlhack.pod b/pod/perlhack.pod
index d559261..52ecaca 100644
--- a/pod/perlhack.pod
+++ b/pod/perlhack.pod
@@ -61,7 +61,8 @@ writes the file as a MIME attachment, and sends it with a meaningful
 subject:
 
   % git format-patch -1 --attach
-  % ./perl -Ilib utils/perlbug -s "[PATCH] $(git log -1 --oneline HEAD)" -f 0001-*.patch
+  % ./perl -Ilib utils/perlbug -s "[PATCH] $(
+        git log -1 --oneline HEAD)" -f 0001-*.patch
 
 The perlbug program will ask you a few questions about your email
 address and the patch you're submitting.  Once you've answered them it

--------------1.8.1.2--


@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2013

From @Smylers

0005-Suggest-reading-blead-s-Super-Quick-Patch-Guide.patch
From 2cacede711429294251748663b526eb90f70dba5 Mon Sep 17 00:00:00 2001
From: Smylers <Smylers@stripey.com>
Date: Wed, 4 Sep 2013 11:37:58 +0100
Subject: [PATCH 5/8] Suggest reading blead's Super Quick Patch Guide
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------1.8.1.2"

This is a multi-part message in MIME format.
--------------1.8.1.2
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


The Super Quick Patch Guide has been improved several times. Suggest that
a patcher looks at the latest version in the check out of blead they've
just made, in case that's been improved since whichever released version
they were reading.

This has caught me out before: I've done something sub-optimal (for me or
those reviewing my patches) by diligently following out-of-date
instructions.
---
 pod/perlhack.pod | 7 +++++++
 1 file changed, 7 insertions(+)


--------------1.8.1.2
Content-Type: text/x-patch; name="0005-Suggest-reading-blead-s-Super-Quick-Patch-Guide.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0005-Suggest-reading-blead-s-Super-Quick-Patch-Guide.patch"

diff --git a/pod/perlhack.pod b/pod/perlhack.pod
index 52ecaca..9c0e59a 100644
--- a/pod/perlhack.pod
+++ b/pod/perlhack.pod
@@ -29,6 +29,13 @@ with the following command:
 
   % git clone git://perl5.git.perl.org/perl.git perl
 
+=item * Ensure you're following the latest advice
+
+In case the advice in this guide has been updated recently, read the
+latest version directly from the perl source:
+
+  % perldoc pod/perlhack.pod
+
 =item * Make your change
 
 Hack, hack, hack.

--------------1.8.1.2--


@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2013

From @Smylers

0006-Resetting-a-check-out-in-Super-Quick-Patch-Guide.patch
From 1147cf5ff4886e07a1456fc2dc44a672cacdc939 Mon Sep 17 00:00:00 2001
From: Smylers <Smylers@stripey.com>
Date: Wed, 4 Sep 2013 11:58:34 +0100
Subject: [PATCH 6/8] Resetting a check out in Super Quick Patch Guide
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------1.8.1.2"

This is a multi-part message in MIME format.
--------------1.8.1.2
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


Add advice so that somebody wishing to submit a second patch doesn't need
to throw away their perl check-out and start again.

Not knowing the git clean step caught me out, and meant perl wouldn't
build for me. Nicholas helped me out. Adding this to the guide will
hopefully save Nicholas from having to repeat that for others (especially
since others may not be fortunate enough to have Nicholas handily seated
next to them at the point they encounter it).

(The non-building was because some things in the repository had been
re-arranged since my previous patch (several months earlier), and the
latest build was getting confused by some files left over from a
pre-re-arragned build.)

The git clean step will also remove the first 0001-*.patch file, avoiding
the problem of there being two files matching that glob when attaching the
second patch.
---
 pod/perlhack.pod | 11 +++++++++++
 1 file changed, 11 insertions(+)


--------------1.8.1.2
Content-Type: text/x-patch; name="0006-Resetting-a-check-out-in-Super-Quick-Patch-Guide.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0006-Resetting-a-check-out-in-Super-Quick-Patch-Guide.patch"

diff --git a/pod/perlhack.pod b/pod/perlhack.pod
index 9c0e59a..6db7e6e 100644
--- a/pod/perlhack.pod
+++ b/pod/perlhack.pod
@@ -80,6 +80,17 @@ will submit your patch via email.
 The porters appreciate the time you spent helping to make Perl better.
 Thank you!
 
+=item * Next time
+
+The next time you wish to make a patch, you need to start from the
+latest perl in a pristine state. Check you don't have any local changes
+or added files in your perl check-out which you wish to keep, then run
+these commands:
+
+  % git pull
+  % git reset --hard origin/blead
+  % git clean -dxf
+
 =back
 
 =head1 BUG REPORTING

--------------1.8.1.2--


@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2013

From @Smylers

0007-Multiple-commits-in-Super-Quick-Patch-Guide.patch
From 58cdd82f332082dd613b18e1af99ab1677a8d5d1 Mon Sep 17 00:00:00 2001
From: Smylers <Smylers@stripey.com>
Date: Wed, 4 Sep 2013 12:37:36 +0100
Subject: [PATCH 7/8] Multiple commits in Super Quick Patch Guide
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------1.8.1.2"

This is a multi-part message in MIME format.
--------------1.8.1.2
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


How to use perlbug when a change is a series of commits, not a single
commit.

This is the advice RJBS gave me over IRC. Including it in the guide should
avoid him having to repeat the advice to others.
---
 pod/perlhack.pod | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)


--------------1.8.1.2
Content-Type: text/x-patch; name="0007-Multiple-commits-in-Super-Quick-Patch-Guide.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0007-Multiple-commits-in-Super-Quick-Patch-Guide.patch"

diff --git a/pod/perlhack.pod b/pod/perlhack.pod
index 6db7e6e..9e61656 100644
--- a/pod/perlhack.pod
+++ b/pod/perlhack.pod
@@ -75,6 +75,23 @@ The perlbug program will ask you a few questions about your email
 address and the patch you're submitting.  Once you've answered them it
 will submit your patch via email.
 
+If your changes are in multiple commits, generate a patch for each of
+them:
+
+  % git format-patch origin/blead --attach
+
+Run perlbug without any attachments:
+
+  % ./perl -Ilib utils/perlbug
+
+Follow the prompts, picking a subject that summarizes your changes
+overall and has "[PATCH]" at the beginning. Describe your changes in
+the editor window that opens.  Instead of sending the report, press f
+to save the message to a file, then quit.
+
+Now create an email using the headers and body from the
+perlbug-generated file, and attach your patches.
+
 =item * Thank you
 
 The porters appreciate the time you spent helping to make Perl better.

--------------1.8.1.2--


@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2013

From @Smylers

0008-Mutt-hint-in-Super-Quick-Patch-Guide.patch
From 3434e8b693abf56b8344eec9859efe16e2e891f0 Mon Sep 17 00:00:00 2001
From: Smylers <Smylers@stripey.com>
Date: Wed, 4 Sep 2013 12:44:46 +0100
Subject: [PATCH 8/8] Mutt hint in Super Quick Patch Guide
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------1.8.1.2"

This is a multi-part message in MIME format.
--------------1.8.1.2
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


When sending an email manually so it can have multiple patches, point out
that Mutt can construct the email for you.

Obviously this isn't as generally relevant as the rest of the Guide, since
patchers will use many different mail clients. But it's a significant boon
for those who do use Mutt, and a very short addition to the Guide.

Mutt is singled out simply because it has this functionality; I suspect
that most other widely used mail clients don't.
---
 pod/perlhack.pod | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)


--------------1.8.1.2
Content-Type: text/x-patch; name="0008-Mutt-hint-in-Super-Quick-Patch-Guide.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0008-Mutt-hint-in-Super-Quick-Patch-Guide.patch"

diff --git a/pod/perlhack.pod b/pod/perlhack.pod
index 9e61656..84d8a6d 100644
--- a/pod/perlhack.pod
+++ b/pod/perlhack.pod
@@ -90,7 +90,10 @@ the editor window that opens.  Instead of sending the report, press f
 to save the message to a file, then quit.
 
 Now create an email using the headers and body from the
-perlbug-generated file, and attach your patches.
+perlbug-generated file, and attach your patches. If you use Mutt, this
+command will do that:
+
+  % mutt -H perlbug.rep -a *.patch
 
 =item * Thank you
 

--------------1.8.1.2--


@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2013

From @cpansprout

On Wed Sep 04 04​:59​:32 2013, smylers@​stripey.com wrote​:

This is a bug report for perl from smylers@​stripey.com,
generated with the help of perlbug 1.39 running under perl 5.19.4.

-----------------------------------------------------------------
I'm suggesting a few tweaks to the Super Quick Patch Guide, based on
my
experiences of using it to submit patches.

There is the risk of the issues MJD discusses here, that adding more
text to the guide makes it less-quick and overall less useful​:
http​://blog.plover.com/prog/featurism.html

My suggested additions are all things that would've helped me (and
saved
the time of Porters I then had to ask for assistance) had they been in
the Guide.

Thank you.

One minor quibble​:

+If your changes are in multiple commits, generate a patch for each of
+them​:
+
+ % git format-patch origin/blead --attach
+

It’s actually slightly easier if the patches are all in one file​:

% git format-patch origin/blead --attach --stdout > patches.txt

But your suggestions are nevertheless an improvement, so I don’t mind if
they go in unchanged.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2013

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2013

From @jkeenan

On Wed Sep 04 04​:59​:32 2013, smylers@​stripey.com wrote​:

This is a bug report for perl from smylers@​stripey.com,
generated with the help of perlbug 1.39 running under perl 5.19.4.

-----------------------------------------------------------------
I'm suggesting a few tweaks to the Super Quick Patch Guide, based on
my
experiences of using it to submit patches.

There is the risk of the issues MJD discusses here, that adding more
text to the guide makes it less-quick and overall less useful​:
http​://blog.plover.com/prog/featurism.html

My suggested additions are all things that would've helped me (and
saved
the time of Porters I then had to ask for assistance) had they been in
the Guide.

All patches applied in a series of eight commits. I removed trailing
whitespace from each and added the RT # to each commit message. I
performed a few small copy edits. I had to add the revisions from patch
0008 manually, as earlier patches had caused line counts to change
slightly and this prevented 0008 from applying cleanly.

Thank you for the patches. Closing ticket.

jimk

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2013

@jkeenan - Status changed from 'open' to 'resolved'

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2013

From @Smylers

Father Chrysostomos via RT writes​:

It’s actually slightly easier if the patches are all in one file​:

Thank you for that.

Further patch attached to say that, which makes the instructions much
simpler.

(Please let me know if I should instead create a new bug for this patch,
since the first batch have been applied and this bug closed.)

Cheers

Smylers
--
Stop drug companies hiding negative research results.
Sign the AllTrials petition to get all clinical research results published.
Read more​: http​://www.alltrials.net/blog/the-alltrials-campaign/

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2013

From @Smylers

0001-Multiple-commits-in-1-attachment-in-Patch-Guide.patch
From 59148d0a8789637230aa1d0f3f3bdcb264d63cd4 Mon Sep 17 00:00:00 2001
From: Smylers <Smylers@stripey.com>
Date: Thu, 5 Sep 2013 09:07:23 +0100
Subject: [PATCH] Multiple commits in 1 attachment in Patch Guide
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------1.8.1.2"

This is a multi-part message in MIME format.
--------------1.8.1.2
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


In the Super Quick Patch Guide, if a change involves multiple commits, put
them all in a single perlbug attachment.

Father Chrysostomos says it's easier like this, in a comment on
RT #119599.

It simplifies the instructions, and avoiding the need to mention mail
clients.
---
 pod/perlhack.pod | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)


--------------1.8.1.2
Content-Type: text/x-patch; name="0001-Multiple-commits-in-1-attachment-in-Patch-Guide.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0001-Multiple-commits-in-1-attachment-in-Patch-Guide.patch"

diff --git a/pod/perlhack.pod b/pod/perlhack.pod
index 94fd963..ff685d2 100644
--- a/pod/perlhack.pod
+++ b/pod/perlhack.pod
@@ -63,8 +63,8 @@ sentence.  For example, "Fixed spelling errors in perlhack.pod".
 The next step is to submit your patch to the Perl core ticket system
 via email.
 
-Assuming your patch consists of a single git commit, the following
-writes the file as a MIME attachment, and sends it with a meaningful
+If your changes are in a single git commit, run the following commands
+to write the file as a MIME attachment and send it with a meaningful
 subject:
 
   % git format-patch -1 --attach
@@ -75,25 +75,14 @@ The perlbug program will ask you a few questions about your email
 address and the patch you're submitting.  Once you've answered them it
 will submit your patch via email.
 
-If your changes are in multiple commits, generate a patch for each of
-them:
+If your changes are in multiple commits, generate a patch file
+containing them all, and attach that:
 
-  % git format-patch origin/blead --attach
+  % git format-patch origin/blead --attach --stdout > patches
+  % ./perl -Ilib utils/perlbug -f patches
 
-Run perlbug without any attachments:
-
-  % ./perl -Ilib utils/perlbug
-
-Follow the prompts, picking a subject that summarizes your changes
-overall and has "[PATCH]" at the beginning. Describe your changes in the
-editor window that opens.  Instead of sending the report, press 'f' to
-save the message to a file, then quit.
-
-Now create an email using the headers and body from the
-perlbug-generated file, and attach your patches. If you use Mutt, this
-command will do that:
-
-  % mutt -H perlbug.rep -a *.patch
+When prompted, pick a subject that summarizes your changes overall and
+has "[PATCH]" at the beginning.
 
 =item * Thank you
 

--------------1.8.1.2--


@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2013

From @Smylers

James E Keenan via RT writes​:

On Wed Sep 04 04​:59​:32 2013, smylers@​stripey.com wrote​:

I'm suggesting a few tweaks to the Super Quick Patch Guide, based on
my experiences of using it to submit patches.

All patches applied in a series of eight commits. I removed trailing
whitespace from each

Actually, I think those were load-bearing trailing spaces, as surprising
as that sounds.

perlhack.pod is written with two spaces between sentences (this may be
policy in other pod files too; I haven't checked).

It also has a comment at the top saying to use Porting/podtidy on it for
consistency, so I did. When a place where podtidy wishes to wrap a
paragraph happens to be the end of a sentence, there will be two spaces
there. The second one will become the line-break, with podtidy leaving
the first one at the end of the preceding line.

Deleting trailing spaces left there by podtidy effectively prevents
podtidy from being idempotent. In some cases the first word of the
sentence that begins immediately after the line-break will happen to be
only one character too long to fit on to the preceding line. If you
delete the trailing spaces, podtidy will now find there's enough room to
move such words on to the end of the previous line. But now, despite
being a sentence break in the middle of a line, there will only be one
space there (because you deleted the other one).

You can fix this, manually restoring the second space. At which point
you're back where you started​: the next run of podtidy will detect that
that line is one character too long, and reformat it, leaving a trailing
space.

To see this, run podtidy on perlhack.pod as of c17e15d then do a git
diff and note that podtidy has found things to reformat (which it didn't
in the patches I submitted with the trailing spaces included). Also see
that its changes result in sentences with only one space after them, and
that that's inconsistent with other sentence breaks in the file.

If you wish to have a strict policy of no trailing spaces in the pod
source then you need to also do one of these​:

* Remove the instruction saying the file should be formatted with
  podtidy.

* Enhance Pod​::Tidy to be able to handle formatting with 2 spaces
  between sentences without ever leaving trailing spaces.

* Reformat perlhack to use a single space between all sentences (‘French
  spacing’) consistently throughout.

Please can you do one of the above, or add the trailing spaces back in
so that the next time somebody runs podtidy it doesn't make the
formatting inconsistent?

(And also because it seems good manners to leave the file in a state
that podtidy is happy with, so the next person making changes isn't
surprised by having podtidy reformat parts they haven't changed.)

and added the RT # to each commit message.

Is that generally desirable for patches?

If so, should the Super Quick Patch Guide be changed to say to first
make a perlbug submission without any attachments, then --amend your
commit to include the RT number, then make the patch and attach it to
the ticket?

I had to add the revisions from patch 0008 manually,

It looks like in doing so you lost the first line of the commit message
(the one-line summary, which in the patches is in the Subject​: header).
It doesn't really matter though, because none of the content of that
patch is needed now Father Chrysostomos has pointed out how to get
multiple commits into a single file; my latest patch removes all the
text added in 0008.

Cheers

Smylers
--
Stop drug companies hiding negative research results.
Sign the AllTrials petition to get all clinical research results published.
Read more​: http​://www.alltrials.net/blog/the-alltrials-campaign/

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2013

From @nwc10

On Thu Sep 05 02​:42​:05 2013, smylers@​stripey.com wrote​:

If you wish to have a strict policy of no trailing spaces in the pod
source then you need to also do one of these​:

* Remove the instruction saying the file should be formatted with
podtidy.

* Enhance Pod​::Tidy to be able to handle formatting with 2 spaces
between sentences without ever leaving trailing spaces.

* Reformat perlhack to use a single space between all sentences (‘French
spacing’) consistently throughout.

Please can you do one of the above, or add the trailing spaces back in
so that the next time somebody runs podtidy it doesn't make the
formatting inconsistent?

Of these, I think that the best solution would be to enhance
Pod​::Tidy to handle formatting with 2 spaces without leaving trailing
spaces.

In that, I'd like to us to be able to avoid trailing spaces, and it
would be annoying to say "we can't use 2 space style because one of
our tools isn't up to it" given that at least one other tool
(emacs, reflowing paragraphs) gets confused if you *don't* use 2
space style.

I don't want to volunteer myself to be the person to contact the
author of Pod​::Tidy to see if he's amenable to this.

(And also because it seems good manners to leave the file in a state
that podtidy is happy with, so the next person making changes isn't
surprised by having podtidy reformat parts they haven't changed.)

and added the RT # to each commit message.

Is that generally desirable for patches?

I think that it's useful.

If so, should the Super Quick Patch Guide be changed to say to first
make a perlbug submission without any attachments, then --amend your
commit to include the RT number, then make the patch and attach it to
the ticket?

but I don't think that this is a good idea. In that, it makes life
rather too complex for the otherwise casual contributor. If people
know what they are doing, and want to help, then it's useful. I think
that writing the "lure people in" instructions with it just adds a
hurdle.

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2013

From @jkeenan

On 9/5/13 5​:41 AM, Smylers wrote​:

James E Keenan via RT writes​:

On Wed Sep 04 04​:59​:32 2013, smylers@​stripey.com wrote​:

I'm suggesting a few tweaks to the Super Quick Patch Guide, based on
my experiences of using it to submit patches.

All patches applied in a series of eight commits. I removed trailing
whitespace from each

Actually, I think those were load-bearing trailing spaces, as surprising
as that sounds.

perlhack.pod is written with two spaces between sentences (this may be
policy in other pod files too; I haven't checked).

It also has a comment at the top saying to use Porting/podtidy on it for
consistency, so I did. When a place where podtidy wishes to wrap a
paragraph happens to be the end of a sentence, there will be two spaces
there. The second one will become the line-break, with podtidy leaving
the first one at the end of the preceding line.

To be honest, until your post I was completely unaware of the existence
of podtidy. It is referenced in '=pod' comments at the top of about 8
files under pod/, but most of our pod does not appear to have been
consistently tidied.

What I do know is that git frequently warns when a commit is made that
creates trailing whitespace. Getting that warning in one of the first
of your patches is what led me to check for trailing whitespace in each
of the subsequent patches.

Deleting trailing spaces left there by podtidy effectively prevents
podtidy from being idempotent. In some cases the first word of the
sentence that begins immediately after the line-break will happen to be
only one character too long to fit on to the preceding line. If you
delete the trailing spaces, podtidy will now find there's enough room to
move such words on to the end of the previous line. But now, despite
being a sentence break in the middle of a line, there will only be one
space there (because you deleted the other one).

You can fix this, manually restoring the second space. At which point
you're back where you started​: the next run of podtidy will detect that
that line is one character too long, and reformat it, leaving a trailing
space.

To see this, run podtidy on perlhack.pod as of c17e15d then do a git
diff and note that podtidy has found things to reformat (which it didn't
in the patches I submitted with the trailing spaces included). Also see
that its changes result in sentences with only one space after them, and
that that's inconsistent with other sentence breaks in the file.

If you wish to have a strict policy of no trailing spaces in the pod
source then you need to also do one of these​:

* Remove the instruction saying the file should be formatted with
podtidy.

* Enhance Pod​::Tidy to be able to handle formatting with 2 spaces
between sentences without ever leaving trailing spaces.

* Reformat perlhack to use a single space between all sentences (‘French
spacing’) consistently throughout.

Please can you do one of the above, or add the trailing spaces back in
so that the next time somebody runs podtidy it doesn't make the
formatting inconsistent?

Since there are various alternatives here, I think that there will have
to be discussion on list and a decision by the pumpking.

(And also because it seems good manners to leave the file in a state
that podtidy is happy with, so the next person making changes isn't
surprised by having podtidy reformat parts they haven't changed.)

and added the RT # to each commit message.

Is that generally desirable for patches?

I've seen it in a lot of patches, and it facilitates archival work. But ...

If so, should the Super Quick Patch Guide be changed to say to first
make a perlbug submission without any attachments, then --amend your
commit to include the RT number, then make the patch and attach it to
the ticket?

... no, that's overly cumbersome and might discourage first-time or
occasional patch submitters. It's something I think we should leave to
the discretion of committers.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2013

@jkeenan - Status changed from 'resolved' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2013

From @jkeenan

Since it appears there will be further discussion in this ticket, I am
re-opening it and un-taking it.

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2013

From @cpansprout

On Thu Sep 05 03​:09​:22 2013, nicholas wrote​:

On Thu Sep 05 02​:42​:05 2013, smylers@​stripey.com wrote​:

and added the RT # to each commit message.

Is that generally desirable for patches?

I think that it's useful.

Unfortunately, in the ‘[perl #xxxx]’ format git-am will strip it out,
unless the Subject line is moved down to the body. So this gets a bit
tricky. I usually edit the patches manually (add a couple of line
breaks) before feeding them to git-am.

Sometimes I feel like writing a patch-application tool (under Porting/)
that will actually round-trip properly, unlike git-am.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2013

From @cpansprout

On Thu Sep 05 01​:49​:03 2013, smylers@​stripey.com wrote​:

Father Chrysostomos via RT writes​:

It’s actually slightly easier if the patches are all in one file​:

Thank you for that.

Further patch attached to say that, which makes the instructions much
simpler.

Thank you for going to the trouble to clarify the instructions. Applied
as 84788b0.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2013

@cpansprout - Status changed from 'open' to 'resolved'

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2013

From @cpansprout

On Thu Sep 05 03​:57​:53 2013, jkeen@​verizon.net wrote​:

What I do know is that git frequently warns when a commit is made that
creates trailing whitespace. Getting that warning in one of the first
of your patches is what led me to check for trailing whitespace in each
of the subsequent patches.

I’ve never understood why that warning is useful. Who cares about
trailing whitespace? Why waste time removing it?

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2013

From @Smylers

James E Keenan writes​:

To be honest, until your post I was completely unaware of the
existence of podtidy.

I was also unaware of it until yesterday. (It might've been better for
all of us if it'd stayed that way.)

Please can you do one of the [ways of no longer requiring
load-bearing trailing spaces], or add the trailing spaces back in so
that the next time somebody runs podtidy it doesn't make the
formatting inconsistent?

Since there are various alternatives here, I think that there will
have to be discussion on list

OK.

and a decision by the pumpking.

Wow, I had no idea this would be so controversial.

My sincere apologies to anybody who'd been planning on doing something
useful today, and instead found themselves spending time discussing
trailing space characters.

Father Chrysostomos via RT writes​:

Applied as 84788b0.

You also re-resolved the ticket. Give Jim had re-opened it for the
trailing spaces matter, I think he'd want it to stay open.

Smylers
--
Stop drug companies hiding negative research results.
Sign the AllTrials petition to get all clinical research results published.
Read more​: http​://www.alltrials.net/blog/the-alltrials-campaign/

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2013

From @cpansprout

On Thu Sep 05 08​:28​:56 2013, smylers@​stripey.com wrote​:

Father Chrysostomos via RT writes​:

Applied as 84788b0.

You also re-resolved the ticket. Give Jim had re-opened it for the
trailing spaces matter, I think he'd want it to stay open.

OK, I misunderstood his reason. I’ll reopen it.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2013

@cpansprout - Status changed from 'resolved' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Sep 10, 2013

From @tonycoz

On Thu Sep 05 06​:19​:53 2013, sprout wrote​:

On Thu Sep 05 03​:57​:53 2013, jkeen@​verizon.net wrote​:

What I do know is that git frequently warns when a commit is made that
creates trailing whitespace. Getting that warning in one of the first
of your patches is what led me to check for trailing whitespace in each
of the subsequent patches.

I’ve never understood why that warning is useful. Who cares about
trailing whitespace? Why waste time removing it?

Who cares about the number of spaces after periods? Why waste time
making sure there's always 2?

Tony

@p5pRT
Copy link
Author

p5pRT commented Sep 10, 2013

From @cpansprout

On Mon Sep 09 18​:33​:18 2013, tonyc wrote​:

On Thu Sep 05 06​:19​:53 2013, sprout wrote​:

On Thu Sep 05 03​:57​:53 2013, jkeen@​verizon.net wrote​:

What I do know is that git frequently warns when a commit is made
that
creates trailing whitespace. Getting that warning in one of the
first
of your patches is what led me to check for trailing whitespace in
each
of the subsequent patches.

I’ve never understood why that warning is useful. Who cares about
trailing whitespace? Why waste time removing it?

Who cares about the number of spaces after periods? Why waste time
making sure there's always 2?

It affects the pod2man output.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Sep 10, 2013

From @Smylers

Tony Cook via RT writes​:

On Thu Sep 05 06​:19​:53 2013, sprout wrote​:

On Thu Sep 05 03​:57​:53 2013, jkeen@​verizon.net wrote​:

What I do know is that git frequently warns when a commit is made
that creates trailing whitespace. Getting that warning in one of
the first of your patches is what led me to check for trailing
whitespace in each of the subsequent patches.

I’ve never understood why that warning is useful. Who cares about
trailing whitespace? Why waste time removing it?

Who cares about the number of spaces after periods?

Apparently the original author of perlhack did. Given they were
consistent before my edits, it seems disrespectful to recklessly make
the output no longer consistent, as a side-effect of how the edits were
applied.

Obviously it's a tiny thing, but it is nonetheless a regression, and I
think we should avoid regressions, especially accidental ones.

Why waste time making sure there's always 2?

If it's decreed that having 1 space at the end of sentences in this
document is acceptable, then tweaking it to use 1 space throughout the
document would be straightforward. This would be a one-off change​: the
spacing would then be stable throughout future edits, applications of
podtidy, and removal of trailing spaces. That was one of the options I
listed for resolving this issue.

Given how trivial it would be to consistently use 1 space, I don't think
there's an advantage to messily using a mixture of 1 and 2 spaces.

Smylers
--
Stop drug companies hiding negative research results.
Sign the AllTrials petition to get all clinical research results published.
Read more​: http​://www.alltrials.net/blog/the-alltrials-campaign/

@p5pRT
Copy link
Author

p5pRT commented Sep 10, 2013

From @cpansprout

On Tue Sep 10 01​:46​:53 2013, smylers@​stripey.com wrote​:

If it's decreed that having 1 space at the end of sentences in this
document is acceptable, then tweaking it to use 1 space throughout the
document would be straightforward. This would be a one-off change​: the
spacing would then be stable throughout future edits, applications of
podtidy, and removal of trailing spaces. That was one of the options I
listed for resolving this issue.

Given how trivial it would be to consistently use 1 space, I don't think
there's an advantage to messily using a mixture of 1 and 2 spaces.

One more consideration. The fact that the docs as rendered by pod2man
used a mixture of double and single spaces bothered me, because it was
not as professional-looking as it could be.

So I did my research. Originally, all the pods written by Larry had two
spaces after fullstops. He also wrote pod2man under the assumption that
that was standard. Hence, it treats a dot at the end of the line within
a paragraph as implicitly having two spaces after it, even if there are
none.

So, if you change a documented to have only one space at the end of a
sentence, it will be rendered with two spaces whenever that sentence
ends at the end of a line.

If you really want consistent *rendering* with one space after each dot,
then dots at EOL in mid-paragraph should to be avoided.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Sep 10, 2013

From @khwilliamson

On 09/10/2013 09​:29 AM, Father Chrysostomos via RT wrote​:

On Tue Sep 10 01​:46​:53 2013, smylers@​stripey.com wrote​:

If it's decreed that having 1 space at the end of sentences in this
document is acceptable, then tweaking it to use 1 space throughout the
document would be straightforward. This would be a one-off change​: the
spacing would then be stable throughout future edits, applications of
podtidy, and removal of trailing spaces. That was one of the options I
listed for resolving this issue.

Given how trivial it would be to consistently use 1 space, I don't think
there's an advantage to messily using a mixture of 1 and 2 spaces.

One more consideration. The fact that the docs as rendered by pod2man
used a mixture of double and single spaces bothered me, because it was
not as professional-looking as it could be.

So I did my research. Originally, all the pods written by Larry had two
spaces after fullstops. He also wrote pod2man under the assumption that
that was standard. Hence, it treats a dot at the end of the line within
a paragraph as implicitly having two spaces after it, even if there are
none.

So, if you change a documented to have only one space at the end of a
sentence, it will be rendered with two spaces whenever that sentence
ends at the end of a line.

If you really want consistent *rendering* with one space after each dot,
then dots at EOL in mid-paragraph should to be avoided.

For the past many centuries, documents have been typically written so
that there is more space between sentences than there is after an
abbreviation (terminated with a period) mid-sentence.

Formatters were designed to interpret two spaces after a period as
indicating an end of sentence, and therefore to more widely space them.
  This was already standard practice in typing on an actual typewriter.
  I haven't seen research on this, but I'd bet that it leads to faster
reading speeds. It may be that younger brains, having grown up with
texts like "C U L8TR" don't respond the way mine does.

The bottom line is that there are people who care about having two
spaces after end-of-sentence periods, and I'm one; and there are
formatters which care too, and do a better job if this convention is
followed.

@p5pRT
Copy link
Author

p5pRT commented Sep 10, 2013

From @jkeenan

On Tue Sep 10 09​:02​:20 2013, public@​khwilliamson.com wrote​:

For the past many centuries, documents have been typically written so
that there is more space between sentences than there is after an
abbreviation (terminated with a period) mid-sentence.

Formatters were designed to interpret two spaces after a period as
indicating an end of sentence, and therefore to more widely space them.
This was already standard practice in typing on an actual typewriter.

Since we're waxing historical, let me wax from personal history ...

When I taught myself to type (on a 1947 Olivetti manual, passed to me by
my grandfather, which I still own and occasionally use) in, IIRC,
freshman year of high school, I learned to type two spacebars at the
conclusion of each sentence.

When I paid my way through graduate school the first time by working as
a word processor in a legal firm (U.S. v. IBM -- the *second* U.S. v.
IBM, for those who are keeping score), this practice was reinforced. I
would look through tens of thousands of pages of trial transcripts, all
typed on IBM Selectrics or Mag Cards, and always see two spacebars at
the end of sentences.

However, when I became a typesetter several years later, I had to
unlearn this practice on my first day on the job. In typesetting,
whether in hand type, hot metal or cold type, one *spaceband* at the end
of a sentence was the rule.

Sadly, I haven't set type in many years -- though that experience taught
me the basics of programming, something which enabled me to pick up Perl
quickly several jobs and a couple of professions later.

My point​: Both single and double spaces at the end of sentences have
long legacies -- just in different contexts.

And now, back our ongoing discussion ...

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Sep 13, 2013

From @nwc10

On Tue, Sep 10, 2013 at 04​:04​:37PM -0700, James E Keenan via RT wrote​:

On Tue Sep 10 09​:02​:20 2013, public@​khwilliamson.com wrote​:

For the past many centuries, documents have been typically written so
that there is more space between sentences than there is after an
abbreviation (terminated with a period) mid-sentence.

Formatters were designed to interpret two spaces after a period as
indicating an end of sentence, and therefore to more widely space them.
This was already standard practice in typing on an actual typewriter.

Including pod2man

And now, back our ongoing discussion ...

So either we change

1) Pod​::Tidy to treat two spaces the way we'd like
2) pod2man to treat spaces the way we like

or we stick with a mess.

Personally, I'd prefer someone to submit a patch to Pod​::Tidy which enables
it to handle 2 spaces nicely without leaving trailing spaces.

(Because the git tools are coded to treat treat trailing spaces as "ugly",
and we're not going to be able to change that either.*)

Nicholas Clark

* And personally, I agree with them. I believe that it's the only bit of
  whitespace whose existence you can't infer. So it's indeterminate.

@p5pRT
Copy link
Author

p5pRT commented Oct 5, 2013

From @rjbs

On Fri Sep 13 03​:16​:13 2013, nicholas wrote​:

Personally, I'd prefer someone to submit a patch to Pod​::Tidy which enables
it to handle 2 spaces nicely without leaving trailing spaces.

That is my preference, also. In the meantime, let's leave trailing spaces in place to move foward
with specific patches.

--
rjbs

@p5pRT
Copy link
Author

p5pRT commented Nov 13, 2013

From @tonycoz

On Thu Sep 05 04​:56​:24 2013, jkeenan wrote​:

Since it appears there will be further discussion in this ticket, I am
re-opening it and un-taking it.

Should this be a new ticket about Pod-Tidy and pod2man 2 spaces support?

The original patches have been applied.

Tony

@p5pRT
Copy link
Author

p5pRT commented Feb 26, 2017

From @jkeenan

On Wed, 13 Nov 2013 03​:44​:57 GMT, tonyc wrote​:

On Thu Sep 05 04​:56​:24 2013, jkeenan wrote​:

Since it appears there will be further discussion in this ticket, I am
re-opening it and un-taking it.

Should this be a new ticket about Pod-Tidy and pod2man 2 spaces support?

The original patches have been applied.

There's been no further discussion in this ticket in 3-1/4 years, so I'm closing this ticket.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT p5pRT closed this as completed Feb 26, 2017
@p5pRT
Copy link
Author

p5pRT commented Feb 26, 2017

@jkeenan - Status changed from 'open' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant