浏览代码

Adopt the house critic profile, hooks and CLAUDE.md for 5.040

.perlcriticrc is now the strict profile from the packaging-perl skill,
which is for code that declares use 5.040 or later.  The old profile
named 28 policies, and nothing ran it.  The tree failed 9 of them, 271
times over for RegularExpressions::RequireDefault.

The strict profile names 146.  The tree fails 63 of them today, and
those are commented out as #[Name], with no space, so that the tree
passes the rest.  Each comes back in the change that fixes the tree
for it.  This includes ProhibitUnusedDefinitions, which the template
leaves off for a library, because tCMS ships programs.
.preferred_modules.ini stays as it was until PreferredModules comes
back with the template's list.

git-hooks/pre-commit is the template hook.  As before, it tidies what
is staged.  Now it also runs the profile over it, and then the tests
that tests-covering chooses.  git-hooks/post-commit keeps the records
of tests-covering up to date.  The comments that named dzil now say
what is true here.  .tests-covering-map.pl says that no test reads the
documents, Makefile.PL, Installer.mk or the npm files.

Makefile.PL lists, as develop prerequisites, what the hooks and the
profile need, because tCMS has no dist.ini to hold authordep lines.
CLAUDE.md is the template's, with the parts about dzil replaced by
Makefile.PL, Readme.md and design.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
troglodyne-bot 1 周之前
父节点
当前提交
b4e23e1f31
共有 8 个文件被更改,包括 950 次插入 和 47 次删除
  1. 2 0
      .gitignore
  2. 238 21
      .perlcriticrc
  3. 247 0
      .pod_stopwords
  4. 67 0
      .tests-covering-map.pl
  5. 127 0
      CLAUDE.md
  6. 70 5
      Makefile.PL
  7. 33 0
      git-hooks/post-commit
  8. 166 21
      git-hooks/pre-commit

+ 2 - 0
.gitignore

@@ -37,3 +37,5 @@ run/
 *-shm
 *-wal
 .xslate_cache/
+*.swn
+cover_db/

+ 238 - 21
.perlcriticrc

@@ -1,26 +1,59 @@
-# Troglodyne perlcritic policy
+# Troglodyne perlcritic policy, for a distribution that declares use 5.040 or
+# later.
+#
+# There are two of these.  This one is for code that runs on a perl you control,
+# and perlcriticrc.compat is for code that runs on whatever perl is already
+# installed.  The packaging-perl skill asks which you are writing before it
+# copies one, because the answer decides more than this file: it decides the
+# use line in every module and test, and perl: in prereqs.yml.
+#
+# The difference between the two is five policies.  A modern perl refuses what
+# they report, so naming them here would buy nothing:
+#
+#   TestingAndDebugging::RequireUseStrict       strict is on from 5.012
+#   Objects::ProhibitIndirectSyntax             no indirect from 5.036
+#   InputOutput::ProhibitBarewordFileHandles    no bareword_filehandles from
+#   InputOutput::ProhibitBarewordDirHandles       5.038
+#   Modules::RequireEndWithOne                  module_true from 5.038
+#
+# Two more go the same way at 5.042, and are named below because this file
+# assumes only 5.040.  Delete them once your floor is 5.041 or later, which
+# selects the 5.042 bundle:
+#
+#   Variables::ProhibitPerl4PackageNames        no apostrophe as a package
+#                                                 separator from 5.042
+#   CodeLayout::RequireASCII                    source::encoding "ascii" from
+#                                                 5.042, which is stricter than
+#                                                 the policy and not skippable
+#
+# tCMS: a policy commented out as #[Name], with no space, is one that the tree
+# does not pass yet.  The change that fixes the tree for it enables it.
 severity  = 1
 only      = 1
 force     = 0
 verbose   = 1
-#theme     = 
-#include   =
-#exclude   =
 criticism-fatal = 1
 color     = 1
 allow-unsafe = 1
 # Usability
 [Modules::RequirePerlVersion]
-[Documentation::RequirePod]
+#[Documentation::RequirePod]
 # Security
 [ValuesAndExpressions::PreventSQLInjection]
-[CodeLayout::RequireASCII]
-[RegularExpressions::RequireDefault]
-strict = 1
+# Drop this one at 5.041 and later.  It also applies to POD, where the only
+# thing it tends to catch is a contributor's name in the section Dist::Zilla
+# generates, and spelling somebody's name wrong is not a code standard.
+#[CodeLayout::RequireASCII]
+#[RegularExpressions::RequireDefault]
+#strict = 1
+#[RegularExpressions::PreventUselessMetacharacterEscapes]
 # PEBCAK catches
 [BuiltinFunctions::ProhibitDeleteOnArrays]
 [Subroutines::ProhibitAmbiguousFunctionCalls]
 [ValuesAndExpressions::ProhibitSingleArgArraySlice]
+#[ValuesAndExpressions::ProhibitDefinedBeforeLength]
+[Variables::ProhibitUselessVarClearing]
+#[Subroutines::ProhibitTailRecursion]
 [Bangs::ProhibitUselessRegexModifiers]
 [Bangs::ProhibitCommentedOutCode]
 [Tics::ProhibitUseBase]
@@ -29,29 +62,213 @@ strict = 1
 [ValuesAndExpressions::UnexpandedSpecialLiteral]
 [CodeLayout::ProhibitIfIfSameLine]
 [ValuesAndExpressions::ConstantBeforeLt]
-[ValuesAndExpressions::ProhibitDuplicateHashKeys]
+#[ValuesAndExpressions::ProhibitDuplicateHashKeys]
 # any access checks are necessarily a TOCTOU waiting to happen
-[ValuesAndExpressions::ProhibitFiletest_f]
+#[ValuesAndExpressions::ProhibitFiletest_f]
 [ValuesAndExpressions::ProhibitFiletest_rwxRWX]
 # Bikeshed
 [CognitiveComplexity::ProhibitExcessCognitiveComplexity]
 warn_level = 40
 info_level = 30
 [Variables::ProhibitUselessInitialization]
-[PreferredModules]
-config = .preferred_modules.ini
+#[PreferredModules]
+#config = .preferred_modules.ini
 [logicLAB::RequireSheBang]
 formats = #!/usr/bin/env perl
 # Performance
-[logicLAB::ProhibitShellDispatch]
+#[logicLAB::ProhibitShellDispatch]
 [ProhibitPipeOpen]
-[Performance::ProhibitRegexForSimpleSubstring]
+#[Performance::ProhibitRegexForSimpleSubstring]
+#[RegularExpressions::ProhibitRegexToStripAffix]
 [RegularExpressions::ProhibitRegexToChomp]
+[BuiltinFunctions::ProhibitChainingSameListSub]
+[RegularExpressions::ProhibitRegexForStringEquality]
+#[Subroutines::ProhibitUnderscorePrivateSubs]
+#[Subroutines::ProhibitSingleUseLexicalSubs]
+#[ValuesAndExpressions::ProhibitLiteralArithmetic]
+[BuiltinFunctions::ProhibitIndirectBooleanGrep]
+# What ProhibitMutatingListFunctions reports, which this replaces, and a block
+# that changes $_ through a sub that it calls.
+#[ControlStructures::ProhibitMutatingListFunctionsStricter]
+# An if, elsif, else or unless whose block holds no statement, a comment
+# being none.  Loops are left alone, because a loop condition can do the work.
+[ControlStructures::ProhibitEmptyConditionalBlocks]
+# A sub that returns the result of a syscall or an eval, called where nothing
+# looks at the result.  The same functions as RequireCheckedSyscalls below.
+[ErrorHandling::RequireCheckedIndirectResults]
+exclude_functions = print
+#[ProhibitUseLib]
+#[ProhibitUsageSubs]
+#[RequireQwForLiteralLists]
+#[ProhibitNoWarningsRedefine]
+#[RequireFatalWarnings]
+#[ProhibitUnusedImports]
+# Modules imported for a side effect rather than for a symbol, which the policy
+# cannot see.  `re` is here because every file says `use re '/aa'`.  Add your
+# own: IO::Prompter installs prompt() from a custom import rather than @EXPORT,
+# and anything loaded by name from a string is invisible the same way.
+#allow = re
+# Off in a distribution that is only a library: it counts calls from bin/ and
+# lib/, and every sub a library exists to offer is called from neither -- a
+# module with one public sub and no bin/ reports that sub, which is the policy
+# working correctly and saying nothing.  Turn it on for a distribution that also
+# ships programs, and add the authordep line to dist.ini when you do.
+#
+# allow_subs then lists the subs that are called in a way it cannot see: named
+# in a string and called as $obj->$name, handed to something else that calls
+# them, or called only from a directory it does not read.  Name each one and say
+# which, so the next person can tell an exemption from an oversight.
+#
+#[ProhibitUnusedDefinitions]
+#allow_subs =
+# A sub that only one subclass uses, and a sub that two subclasses define where
+# the class above them defines none.  It reads bin/ and lib/ through
+# Perl::Critic::Distribution, as ProhibitUnusedDefinitions does, so the two
+# share one parse.  A library whose subclasses live in other distributions
+# names in allow_subs each hook that only one of its own subclasses uses.
+#[ProhibitMisplacedMethods]
+#[ProhibitPrintSTDERR]
+#[InputOutput::ProhibitRepeatedPrints]
+# Perl::Critic's own policies.  only = 1 loads nothing that is not named, so
+# each of these has to be, and the ones that are absent are absent on purpose.
+#
+# Not here, because something above already does the job better:
+#   CodeLayout::ProhibitQuotedWordLists         RequireQwForLiteralLists
+#   ControlStructures::ProhibitCascadingIfElse  CognitiveComplexity, which
+#   ControlStructures::ProhibitDeepNests          scores nesting and every
+#   Subroutines::ProhibitExcessComplexity         break in flow, where these
+#                                                 count branches
+#   ControlStructures::ProhibitMutatingListFunctions
+#                                               ProhibitMutatingListFunctionsStricter,
+#                                                 which reports all of it, and
+#                                                 a sub that the block calls
+#   InputOutput::ProhibitBacktickOperators      logicLAB::ProhibitShellDispatch
+#   Modules::ProhibitEvilModules                PreferredModules
+#   Subroutines::ProhibitUnusedPrivateSubroutines  ProhibitUnusedDefinitions,
+#                                                 which counts calls from bin/
+#                                                 and lib/, private or not
+#   TestingAndDebugging::RequireUseWarnings     RequireFatalWarnings
+#   ValuesAndExpressions::ProhibitLeadingZeros  Plicease::ProhibitLeadingZeros
+#   Modules::RequireVersionVar                  [PkgVersion] in dist.ini
+#   ErrorHandling::RequireCarping               die with context, and
+#                                                 Carp::Always for the trace
+#
+# Nor the five the language enforces under this file's floor.  The header says
+# which they are, and perlcriticrc.compat is where they live.
+#
+# Documentation::RequirePodSections is out as well: the sections it asks for are
+# the ones Pod::Weaver writes at build, and weaver.ini names them AUTHORS and
+# COPYRIGHT AND LICENSE where the policy wants AUTHOR and LICENSE AND COPYRIGHT.
+#[BuiltinFunctions::ProhibitBooleanGrep]
+[BuiltinFunctions::ProhibitLvalueSubstr]
+#[BuiltinFunctions::ProhibitReverseSortBlock]
+[BuiltinFunctions::ProhibitShiftRef]
+[BuiltinFunctions::ProhibitSleepViaSelect]
+[BuiltinFunctions::ProhibitStringyEval]
+#[BuiltinFunctions::ProhibitStringySplit]
+[BuiltinFunctions::ProhibitUniversalCan]
+[BuiltinFunctions::ProhibitUniversalIsa]
+[BuiltinFunctions::ProhibitVoidGrep]
+#[BuiltinFunctions::ProhibitVoidMap]
+[BuiltinFunctions::RequireBlockGrep]
+[BuiltinFunctions::RequireBlockMap]
+[BuiltinFunctions::RequireGlobFunction]
+[BuiltinFunctions::RequireSimpleSortBlock]
+[ClassHierarchies::ProhibitAutoloading]
+#[ClassHierarchies::ProhibitExplicitISA]
+#[ClassHierarchies::ProhibitOneArgBless]
+[CodeLayout::ProhibitHardTabs]
+[CodeLayout::ProhibitTrailingWhitespace]
+[CodeLayout::RequireConsistentNewlines]
+# [CodeLayout::RequireTidyCode] is off on purpose.  git-hooks/pre-commit runs
+# perltidy over every staged Perl file before it runs this profile, so every
+# commit is tidy whatever this policy would say.  In an editor, the policy only
+# repeated that, and ran perltidy over the whole file on every check.
+[CodeLayout::RequireTrailingCommas]
+[ControlStructures::ProhibitLabelsWithSpecialBlockNames]
+#[ControlStructures::ProhibitNegativeExpressionsInUnlessAndUntilConditions]
+[ControlStructures::ProhibitUnreachableCode]
+[ControlStructures::ProhibitUntilBlocks]
+#[ControlStructures::ProhibitYadaOperator]
+#[Documentation::PodSpelling]
+#spell_command = aspell --lang=en_US list
+#stop_words_file = .pod_stopwords
+[Documentation::RequirePackageMatchesPodName]
+#[ErrorHandling::RequireCheckingReturnValueOfEval]
+#[InputOutput::ProhibitExplicitStdin]
+#[InputOutput::ProhibitInteractiveTest]
+[InputOutput::ProhibitJoinedReadline]
+[InputOutput::ProhibitOneArgSelect]
+[InputOutput::ProhibitReadlineInForLoop]
+[InputOutput::ProhibitTwoArgOpen]
+#[InputOutput::RequireBracedFileHandleWithPrint]
+#[InputOutput::RequireBriefOpen]
+[InputOutput::RequireCheckedOpen]
+#[InputOutput::RequireCheckedSyscalls]
+#exclude_functions = print
+[InputOutput::RequireEncodingWithUTF8Layer]
+[Miscellanea::ProhibitFormats]
+[Miscellanea::ProhibitTies]
+[Miscellanea::ProhibitUnrestrictedNoCritic]
+#[Miscellanea::ProhibitUselessNoCritic]
+[Modules::ProhibitAutomaticExportation]
+#[Modules::ProhibitConditionalUseStatements]
+#[Modules::RequireBarewordIncludes]
+#[Modules::RequireFilenameMatchesPackage]
+[Modules::RequireNoMatchVarsWithUseEnglish]
+#[NamingConventions::Capitalization]
+[RegularExpressions::ProhibitCaptureWithoutTest]
+#[RegularExpressions::ProhibitComplexRegexes]
+#[RegularExpressions::ProhibitEnumeratedClasses]
+#[RegularExpressions::ProhibitFixedStringMatches]
+[RegularExpressions::ProhibitSingleCharAlternation]
+[RegularExpressions::ProhibitUnusedCapture]
+[RegularExpressions::RequireBracesForMultiline]
+#[RegularExpressions::RequireDotMatchAnything]
+#[RegularExpressions::RequireExtendedFormatting]
+[Subroutines::ProhibitAmpersandSigils]
+#[Subroutines::ProhibitBuiltinHomonyms]
+[Subroutines::ProhibitManyArgs]
+[Subroutines::ProhibitNestedSubs]
+#[Subroutines::ProhibitReturnSort]
+[Subroutines::ProhibitSubroutinePrototypes]
+#[Subroutines::ProtectPrivateSubs]
+#[Subroutines::RequireArgUnpacking]
+#[Subroutines::RequireFinalReturn]
+[TestingAndDebugging::ProhibitProlongedStrictureOverride]
+[TestingAndDebugging::RequireTestLabels]
+#[ValuesAndExpressions::ProhibitCommaSeparatedStatements]
+[ValuesAndExpressions::ProhibitComplexVersion]
+[ValuesAndExpressions::ProhibitEscapedCharacters]
+[ValuesAndExpressions::ProhibitLongChainsOfMethodCalls]
+#[ValuesAndExpressions::ProhibitMismatchedOperators]
+[ValuesAndExpressions::ProhibitMixedBooleanOperators]
+[ValuesAndExpressions::ProhibitQuotesAsQuotelikeOperatorDelimiters]
+[ValuesAndExpressions::ProhibitSpecialLiteralHeredocTerminator]
+[ValuesAndExpressions::ProhibitVersionStrings]
+[ValuesAndExpressions::RequireConstantVersion]
+[ValuesAndExpressions::RequireQuotedHeredocTerminator]
+[ValuesAndExpressions::RequireUpperCaseHeredocTerminator]
+[Variables::ProhibitAugmentedAssignmentInDeclaration]
+[Variables::ProhibitConditionalDeclarations]
+# Variables::ProhibitEvilVariables is not listed: it declines to load until it
+# is given variables to prohibit, and nobody has named any.
+#[Variables::ProhibitLocalVars]
+[Variables::ProhibitMatchVars]
+# Drop this one at 5.041 and later, with RequireASCII above.
+[Variables::ProhibitPerl4PackageNames]
+#[Variables::ProhibitReusedNames]
+# Stricter than the core ProhibitUnusedVariables, which reports only a bare
+# `my $x;` and misses a variable that is assigned and never read.
+#[Variables::ProhibitUnusedVarsStricter]
+# A method takes the arguments of its interface, whether it reads them or not.
+#allow_unused_subroutine_arguments = 1
+# Anything that returns a guard, which does its work by being held to the end of
+# its scope: a mock, a mocked file, a change of directory, a lock.
+#allow_if_computed_by = Test::MockModule->new Test::MockFile->file
+[Variables::ProtectPrivateVars]
+[Variables::RequireLexicalLoopIterators]
+#[Variables::RequireLocalizedPunctuationVars]
+[Variables::RequireNegativeIndices]
 # Extensions to perl -c
-[CompileTime]
-# Good idea, but module doesn't install due to bad testsuite...we'll have to fork
-#[Dynamic::ValidateAgainstSymbolTable]
-#at_inc_suffix=lib
-#inspect_required_modules=1
-#inspect_autoloaders=1
-#max_recursion=2
+#[CompileTime]

+ 247 - 0
.pod_stopwords

@@ -0,0 +1,247 @@
+# Words Documentation::PodSpelling takes as spelled right, named in .perlcriticrc.
+# aspell is asked for American English, which is what the POD here is written in,
+# so what is left is vocabulary no dictionary has -- commands, daemons, file and
+# option names, module names -- and the pieces of a token aspell splits at a
+# digit, like the IPv of IPv4.  A misspelling does not belong here: fix it.
+#
+# One word per line, sorted without regard to case, so that adding one is a
+# one-line diff and nobody has to read the whole file to decide whether it is
+# already in it.
+#
+# Put each part of your own name and of the copyright holder's in here, one per
+# line.  PodSpelling reads the AUTHORS and COPYRIGHT sections that Pod::Weaver
+# generates, and a surname is in no dictionary.  The licence words below are
+# from the text weaver.ini puts in the COPYRIGHT section of every module.
+
+accessor
+ACL
+addon
+AIO
+API
+arg
+args
+argv
+arrayref
+autodetect
+autodetected
+autodetecting
+autodetects
+autogen
+backend
+backends
+BackPAN
+backtick
+basename
+bugtracker
+checksummed
+checksumming
+CHECKSUMS
+checksums
+chmod
+chmodded
+chmodding
+chmods
+chown
+chowned
+chowning
+chowns
+chroot
+chrooted
+chrooting
+chroots
+CIDR
+CIDRs
+CLI
+codename
+coderef
+config
+configs
+CPAN
+cpanm
+CPUs
+cron
+crons
+CVE
+DDL
+deduplicate
+deduplicated
+deduplicates
+deduplicating
+dep
+deps
+dev
+DHCP
+dir
+distro
+distros
+DKIM
+DMARC
+DN
+DNS
+dotfiles
+dryrun
+DSN
+dzil
+ECDSA
+ENV
+env
+filehandle
+filesystem
+filetest
+formatter
+formatters
+FQDN
+GiB
+GraphQL
+greppable
+hardcode
+hardcoded
+hardcodes
+hardcoding
+hashref
+heredoc
+hostname
+hotplug
+HTTPS
+IMAP
+INI
+init
+IPs
+IPv
+JSON
+keypair
+keyspace
+KiB
+LDAP
+LDAPS
+LDIF
+lifecycle
+lookups
+loopback
+lowercased
+lowercases
+lowercasing
+LTS
+MACs
+makefile
+makefiles
+memoize
+memoized
+memoizes
+memoizing
+MERCHANTABILITY
+MetaCPAN
+MiB
+misconfigure
+misconfigured
+misconfigures
+misconfiguring
+mistyped
+mtime
+MX
+nameserver
+nameservers
+namespace
+NIC
+NONINFRINGEMENT
+NTP
+OAuth
+OOM
+overcommit
+overcommits
+overcommitted
+overcommitting
+passwordless
+PCI
+PCIe
+PEM
+perlcritic
+perltidy
+PKI
+plaintext
+POSIX
+pragmas
+preflight
+proxied
+proxying
+readdir
+recurse
+recursed
+recurses
+recursing
+regenerable
+renderer
+repo
+repos
+RPC
+RSA
+rsync
+rsynced
+rsyncing
+rsyncs
+ruleset
+runtime
+scaffolded
+scp
+sftp
+SHA
+SMTP
+SRV
+sshd
+SSL
+stdin
+stdout
+Subclasses
+subdir
+subdirectory
+sublicense
+subnet
+substring
+subtest
+sudo
+symlink
+symlinked
+symlinking
+symlinks
+sysctl
+syslog
+systemd
+TCP
+teardown
+templated
+templating
+timestamp
+TLD
+TLDs
+TLS
+TODO
+toolchain
+TXT
+udev
+UDP
+UI
+UID
+umask
+uncached
+unconfigured
+undef
+undefine
+undefines
+undefining
+unlink
+unlinked
+unlinking
+unlinks
+URI
+URIs
+UUID
+validator
+vhost
+vhosts
+VM
+VMs
+VPN
+VPNs
+WAL
+whitespace
+YAML

+ 67 - 0
.tests-covering-map.pl

@@ -0,0 +1,67 @@
+#!/usr/bin/env perl
+use 5.040;
+use warnings FATAL => 'all';
+use re '/aa';
+
+=head1 NAME
+
+.tests-covering-map.pl - which tests reach the files that no test loads, for
+tests-covering
+
+=head1 DESCRIPTION
+
+git-hooks/pre-commit asks tests-covering which tests a commit can break.
+tests-covering follows the Perl that each test loads, and asks this map about
+every other file in the diff.  See "THE MAP" in L<Perl::Tests::Covering>.
+
+The map answers C<q{}>, which is C<NO_TESTS>, for the files of tCMS that
+C<prove -l t/> never reads:
+
+=over
+
+=item Documentation
+
+Markdown, such as F<Readme.md> and F<design.md>, POD in F<.pod> files outside
+F<lib/>, F<CHANGELOG> and F<LICENSE>.
+
+=item The build and the install
+
+F<Makefile.PL>, F<Installer.mk>, F<package.json> and F<package-lock.json>.
+C<make> and C<npm> read them, and no test does.
+
+=item The tools and the hooks
+
+The C<perltidy> and C<perlcritic> configuration and the files that it reads,
+F<.mailmap>, F<.gitignore>, F<.gitattributes>, this map, F<git-hooks/> and
+F<.github/>.
+
+=item The author tests
+
+F<xt/>, which the hooks do not run.
+
+=back
+
+Anything else, it leaves unexplained, and the hook then runs every test.  A
+file that a test reads without loading it, such as a template or a fixture,
+wants a rule here that names the tests that read it.
+
+=cut
+
+# Read by the tools, by make or by people, and by no test.
+my %NO_TESTS = map { $_ => 1 } qw{
+  CHANGELOG LICENSE
+  Makefile.PL Installer.mk package.json package-lock.json
+  .perltidyrc .perlcriticrc .perlcriticrc.compat .pod_stopwords .preferred_modules.ini .preferred_binaries.ini
+  .mailmap .gitignore .gitattributes .tests-covering-map.pl
+};
+
+return sub {
+    my ($path) = @_;
+
+    return q{} if $NO_TESTS{$path};
+    return q{} if $path =~ m{\A(?:git-hooks|[.]github|xt)/}sx;
+    return q{} if $path =~ m{[.]md\z}sx;
+    return q{} if $path =~ m{[.]pod\z}sx && $path !~ m{\Alib/}sx;
+
+    return;
+};

+ 127 - 0
CLAUDE.md

@@ -0,0 +1,127 @@
+# CLAUDE.md
+
+This file is the procedure for work in this repository.  The POD and the files
+named below say what the code is and how it is written.
+
+tCMS is a CMS for perl, run by tPSGI, in which a post can be content of any type.
+Read `design.md` first: it says what a post is, why the routing table is
+data, and where a given change belongs.
+
+## Read the code before you change it
+
+Before you answer a question about the code here, look for something in it, or
+edit it, invoke `perl-slop:reading-perl`.
+
+Most of the code here is older than the conversation about it.  A line that
+looks pointless is usually the scar of something that went wrong once.  The
+reason is in the commit, not in the file.  So read before the first edit, not
+after the tests fail.
+
+The same pass asks whether the code that you want to write is already here,
+under a name that you did not search for.  Two subs that do one job are a
+defect, whichever one is better.
+
+## What perl this runs on
+
+Every module, script and test says `use 5.040`, and `MIN_PERL_VERSION` in
+`Makefile.PL` says the same.  `.perlcriticrc` depends on it.  The header of
+that file names the policies that this version of perl makes unnecessary.  So
+if you raise or lower the floor, change the profile as well as the `use` lines.
+
+## Where it is written down
+
+| | |
+|---|---|
+| `Readme.md` | what tCMS is for, and how to deploy it |
+| `design.md` | the architecture, and where a given change belongs |
+| `.perlcriticrc` | the policies, which are the house style made enforceable |
+| `CHANGELOG` | what changed, per major version |
+| `Makefile.PL` | what has to be installed to run tCMS, and to develop it |
+| `Installer.mk` | the directories and the front-end files that an install needs |
+
+## Finishing a changeset
+
+Before you commit, apply these skills in this order:
+
+1. `perl-slop:data-perl`: make sure that the data is defined, coerced,
+   validated and scoped in the way that perl expects.
+2. `perl-slop:testing-perl`: make sure that each behavior that you added or
+   changed has a test, and that the test is the right kind.
+3. `perl-slop:reviewing-perl`: read the whole diff back against it.  This pass
+   finds a second copy of something that the library already does.  It also
+   finds a call out to the shell, and a comment that belongs in the commit
+   message.
+
+Then run `podchecker` on each changed file.  Run `perl -Ilib -c` on each
+changed script that no test loads, because the tests do not compile it.
+
+The pre-commit hook does the rest.  It tidies the Perl that you staged, runs
+perlcritic on it, and runs the tests that the commit can break, which
+`tests-covering` chooses.  The post-commit hook keeps the records of
+`tests-covering` up to date, in the background.  If a step fails, the hook stops the
+commit and prints the reason.  Do not run `perltidy` or `perlcritic` yourself,
+and do not run the tests to decide whether a change is ready to commit.
+
+The hook names each test that failed.  Its output does not say why.  If a test
+fails, run that file yourself with `-v`, and read the output:
+
+```
+prove -lv t/<file>.t
+```
+
+Do the same to see a new test fail before you fix what it tests.
+
+In each clone that you commit from, install both hooks once.  Git does not
+install them for you, and without the hooks the tree drifts from its style:
+
+```
+cp git-hooks/pre-commit git-hooks/post-commit .git/hooks/
+```
+
+A file that no test loads, such as a fixture, gets its tests from
+`.tests-covering-map.pl`.  A path that the map cannot place runs every test.
+If a commit runs every test but does not touch everything, the map probably
+needs a rule.  Add the rule in the same change, and name the tests that read
+the file.
+
+## When something is slow
+
+Use `perl-slop:profiling-perl`.  Measure before you conclude anything, and
+measure again after a change.  "It is just slow" is not a finding.  A line
+number and a percentage is a finding.
+
+## Installing what development needs
+
+The hooks and `.perlcriticrc` need the develop prerequisites in `Makefile.PL`.
+When `.perlcriticrc` names a policy from another distribution, add that
+distribution there in the same change.
+
+```
+cpanm --installdeps --with-develop .
+```
+
+## Commits and pull requests
+
+Make a branch.  Do not commit to the default branch.
+
+A commit message here says what was wrong and why the change fixes it.  Write
+it in prose and in the imperative mood.  Name the behavior, not the diff, for
+example "Ask the pool whether it takes O_DIRECT, rather than guessing from its
+name".  The message has a job.  `perl-slop:reading-perl` describes a person who
+finds your line with `git blame` two years from now.  The message is the only
+thing that can still tell that person why.  For the same reason, the why goes
+in the message and not in a comment.
+
+After you make sure that something works, say what you ran and what it said.
+A claim that the tests pass needs the line that shows them passing.  A claim
+about your own work needs its evidence too, such as the URL from `gh pr
+create` or the sha from `git push`.  A PR number that nobody can open is worse
+than no number.
+
+If the code of a branch depends on another branch, stack it on that branch.
+If only a test of it depends on the other branch, do not stack it.  A change
+whose tests need a fix from somebody else is still an independent change.
+Open it against the default branch, and say in the description what must
+merge first.  A stacked branch merges into its base, whatever that base is.
+If the base reached the default branch by another route, the stacked change
+reaches nowhere, and nothing says so.

+ 70 - 5
Makefile.PL

@@ -1,4 +1,4 @@
-use strict;
+use 5.040;
 use warnings;
 
 use ExtUtils::MakeMaker;
@@ -6,12 +6,77 @@ use ExtUtils::MakeMaker;
 WriteMakefile(
     VERSION => '0.01',
 
-    # Every module and script declares 'use v5.36' -- signatures without the
-    # experimental warning is what we build on.
-    MIN_PERL_VERSION => '5.036000',
+    # Every module, script and test declares 'use 5.040'.
+    MIN_PERL_VERSION => '5.040000',
     META_MERGE       => {
         "meta-spec" => { version => 2 },
-        resources   => {
+
+        # What the hooks and .perlcriticrc need, which nothing at run time does.
+        # Add a line whenever .perlcriticrc names a policy from another distribution.
+        prereqs => {
+            develop => {
+                requires => {
+                    'Perl::Critic'                                                                   => '0',
+                    'Perl::Tidy'                                                                     => '0',
+                    'Devel::Cover'                                                                   => '0',
+                    'Perl::Tests::Covering'                                                          => '0.002',
+                    'Perl::Critic::Policy::Modules::RequirePerlVersion'                              => '0',
+                    'Perl::Critic::Policy::Documentation::RequirePod'                                => '0',
+                    'Perl::Critic::Policy::ValuesAndExpressions::PreventSQLInjection'                => '0',
+                    'Perl::Critic::Policy::CodeLayout::RequireASCII'                                 => '0',
+                    'Perl::Critic::Policy::RegularExpressions::RequireDefault'                       => '0',
+                    'Perl::Critic::Policy::BuiltinFunctions::ProhibitDeleteOnArrays'                 => '0',
+                    'Perl::Critic::Policy::Subroutines::ProhibitAmbiguousFunctionCalls'              => '0',
+                    'Perl::Critic::Policy::ValuesAndExpressions::ProhibitSingleArgArraySlice'        => '0',
+                    'Perl::Critic::Policy::Bangs::ProhibitUselessRegexModifiers'                     => '0',
+                    'Perl::Critic::Policy::Bangs::ProhibitCommentedOutCode'                          => '0',
+                    'Perl::Critic::Policy::Tics::ProhibitUseBase'                                    => '0',
+                    'Perl::Critic::Policy::Plicease::ProhibitLeadingZeros'                           => '0',
+                    'Perl::Critic::Policy::RegularExpressions::ProhibitEmptyAlternatives'            => '0',
+                    'Perl::Critic::Policy::ValuesAndExpressions::UnexpandedSpecialLiteral'           => '0',
+                    'Perl::Critic::Policy::CodeLayout::ProhibitIfIfSameLine'                         => '0',
+                    'Perl::Critic::Policy::ValuesAndExpressions::ConstantBeforeLt'                   => '0',
+                    'Perl::Critic::Policy::ValuesAndExpressions::ProhibitDuplicateHashKeys'          => '0',
+                    'Perl::Critic::Policy::ValuesAndExpressions::ProhibitFiletest_f'                 => '0',
+                    'Perl::Critic::Policy::ValuesAndExpressions::ProhibitFiletest_rwxRWX'            => '0',
+                    'Perl::Critic::Policy::CognitiveComplexity::ProhibitExcessCognitiveComplexity'   => '0',
+                    'Perl::Critic::Policy::Variables::ProhibitUselessInitialization'                 => '0',
+                    'Perl::Critic::Policy::PreferredModules'                                         => '0',
+                    'Perl::Critic::Policy::logicLAB::RequireSheBang'                                 => '0',
+                    'Perl::Critic::Policy::logicLAB::ProhibitShellDispatch'                          => '0',
+                    'Perl::Critic::Policy::Performance::ProhibitRegexForSimpleSubstring'             => '0',
+                    'Perl::Critic::Policy::RegularExpressions::ProhibitRegexToStripAffix'            => '0',
+                    'Perl::Critic::Policy::RegularExpressions::ProhibitRegexToChomp'                 => '0',
+                    'Perl::Critic::Policy::BuiltinFunctions::ProhibitChainingSameListSub'            => '0',
+                    'Perl::Critic::Policy::RegularExpressions::ProhibitRegexForStringEquality'       => '0',
+                    'Perl::Critic::Policy::Subroutines::ProhibitUnderscorePrivateSubs'               => '0',
+                    'Perl::Critic::Policy::Subroutines::ProhibitSingleUseLexicalSubs'                => '0',
+                    'Perl::Critic::Policy::ValuesAndExpressions::ProhibitLiteralArithmetic'          => '0',
+                    'Perl::Critic::Policy::BuiltinFunctions::ProhibitIndirectBooleanGrep'            => '0',
+                    'Perl::Critic::Policy::ControlStructures::ProhibitMutatingListFunctionsStricter' => '0',
+                    'Perl::Critic::Policy::ControlStructures::ProhibitEmptyConditionalBlocks'        => '0',
+                    'Perl::Critic::Policy::ErrorHandling::RequireCheckedIndirectResults'             => '0',
+                    'Perl::Critic::Policy::CompileTime'                                              => '0',
+                    'Perl::Critic::Policy::ProhibitPipeOpen'                                         => '0',
+                    'Perl::Critic::Policy::ProhibitUseLib'                                           => '0',
+                    'Perl::Critic::Policy::ProhibitUsageSubs'                                        => '0',
+                    'Perl::Critic::Policy::RequireQwForLiteralLists'                                 => '0',
+                    'Perl::Critic::Policy::ProhibitNoWarningsRedefine'                               => '0',
+                    'Perl::Critic::Policy::RequireFatalWarnings'                                     => '0',
+                    'Perl::Critic::Policy::ProhibitUnusedImports'                                    => '0',
+                    'Perl::Critic::Policy::ProhibitMisplacedMethods'                                 => '0',
+                    'Perl::Critic::Policy::ProhibitPrintSTDERR'                                      => '0',
+                    'Perl::Critic::Policy::InputOutput::ProhibitRepeatedPrints'                      => '0',
+                    'Perl::Critic::Policy::ValuesAndExpressions::ProhibitDefinedBeforeLength'        => '0',
+                    'Perl::Critic::Policy::Variables::ProhibitUselessVarClearing'                    => '0.001',
+                    'Perl::Critic::Policy::Subroutines::ProhibitTailRecursion'                       => '0.001',
+                    'Perl::Critic::Policy::RegularExpressions::PreventUselessMetacharacterEscapes'   => '0',
+                    'Perl::Critic::Policy::Variables::ProhibitUnusedVarsStricter'                    => '0',
+                    'Perl::Critic::Policy::ProhibitUnusedDefinitions'                                => '0',
+                },
+            },
+        },
+        resources => {
             repository => {
                 type => 'git',
                 url  => 'git@github.com:Troglodyne-Internet-Widgets/tCMS.git',

+ 33 - 0
git-hooks/post-commit

@@ -0,0 +1,33 @@
+#!/bin/sh
+
+# Bring the records of tests-covering up to the commit just made.
+#
+# git-hooks/pre-commit chooses tests by reading a diff against these records,
+# and a diff describes the files as they were before the change.  So after each
+# commit the records must describe that commit, or the next choice falls back
+# to every test that loaded a changed file, and grows coarser with each commit.
+#
+# In the background, because it runs each stale test under Devel::Cover, and a
+# full refresh takes much longer than a commit should.  Run it by hand to catch
+# up.  It runs the tests the way the pre-commit hook does, with lib/ on the
+# path and nothing else set, so that the records describe the same runs.
+#
+# Install it: cp git-hooks/pre-commit git-hooks/post-commit .git/hooks/
+
+command -v tests-covering >/dev/null 2>&1 || exit 0
+
+# Not while a rebase, or a cherry-pick or revert of several commits, applies
+# them.  git runs this hook for each commit that it applies, and each refresh
+# runs every stale test under Devel::Cover.  A refresh brings the records up to
+# the working tree, however many commits it missed, so the next commit after
+# the rebase catches them up.
+for state in rebase-merge rebase-apply sequencer; do
+    [ -d "$(git rev-parse --git-path "$state")" ] && exit 0
+done
+
+# Without the GIT_ variables that git sets for this hook, for the reason the
+# pre-commit hook gives: the refresh runs the tests, and a test that runs git
+# in a repository of its own would otherwise work in this one.
+unset $(env | sed -n 's/^\(GIT_[A-Za-z0-9_]*\)=.*/\1/p')
+
+tests-covering --refresh --jobs "$(nproc 2>/dev/null || echo 1)" >/dev/null 2>&1 &

+ 166 - 21
git-hooks/pre-commit

@@ -1,61 +1,206 @@
 #!/bin/sh
 
-# Auto-tidy staged Perl before it lands.
+# Tidy staged Perl, judge it against .perlcriticrc, then run the tests, before
+# any of it lands.
+#
+# There is a .perltidyrc, and a tree stays tidy to it only if something makes it.
+# Left to people, it does not: tidying a file you are changing three lines of
+# buries the change in a hundred lines of reformatting, so the reasonable thing
+# is to skip it, and the tree drifts until a mass tidy is the only way back.
+# One commit's worth at a time is small enough that nobody has to weigh it up.
+#
+# perlcritic is here for the other half of that.  Nothing else in tCMS runs the
+# profile, so this is where an objection is about the diff in front of you.
+#
+# There is one profile, .perlcriticrc, and which one that is was decided when
+# the floor was chosen: the packaging-perl skill asks which perl you
+# are targeting and copies the matching profile to that name. So the hook needs
+# no opinion about versions -- the file it reads is already the right one, and
+# changing your floor means replacing that file rather than editing this.
+#
+# Install it: cp git-hooks/pre-commit .git/hooks/
 #
 # A file qualifies three ways:
 #   - a .pm, .pl or .t extension
-#   - a .PL extension, which is how Makefile.PL gets covered
+#   - a .PL extension, which is how a Makefile.PL gets covered
 #   - executable with perl on its shebang line, which is how the extensionless
 #     scripts in bin/ get covered
 #
-# .t is in there because only one of the seven test files happens to be +x,
-# so the shebang rule alone would have tidied that one and let the rest drift.
+# .t is in there because test files are often not executable, and the shebang
+# rule alone would then tidy whichever ones happen to be and let the rest drift.
+#
+# Both passes read the working tree rather than the index, so a file staged in
+# part is judged whole. That is deliberate: perltidy rewrites the file on disk
+# and the hook stages what it wrote, so the working copy is what lands anyway,
+# and two passes disagreeing about which bytes they looked at would be worse to
+# own than this.
+#
+# Both run from the top of the working tree, which is where git runs a hook and
+# the only place they work at all: the profile names .preferred_modules.ini and
+# .pod_stopwords by relative path.
 
 # Redirect output to stderr.
 exec 1>&2
 
-if ! command -v perltidy >/dev/null 2>&1
-then
-    echo "pre-commit: perltidy is not on PATH, skipping the tidy pass."
-    exit 0
-fi
-
 # ACMR so that a file deleted in this commit is not handed to perltidy, which
 # would fail on a path that is no longer there.
 staged=$(git diff --cached --name-only --diff-filter=ACMR)
 
-to_tidy=''
+lib_files=''
+test_files=''
 for file in $staged
 do
     test -f "$file" || continue
 
+    is_perl=''
     case "$file" in
         *.pm|*.pl|*.PL|*.t)
-            to_tidy="$to_tidy $file"
-            continue
+            is_perl=1
             ;;
     esac
 
-    if test -x "$file" && head -n 1 "$file" | grep -Eq '^#!.*perl'
+    if [ -z "$is_perl" ] && test -x "$file" && head -n 1 "$file" | grep -Eq '^#!.*perl'
     then
-        to_tidy="$to_tidy $file"
+        is_perl=1
     fi
+
+    test -n "$is_perl" || continue
+
+    # Whether a file is perl at all, and then which list it goes in.  One case
+    # statement doing both would file t/00-load.t under whichever arm it reached
+    # first, and the .t rule is above the path rule.
+    case "$file" in
+        t/*) test_files="$test_files $file" ;;
+        *)   lib_files="$lib_files $file" ;;
+    esac
 done
 
-# -n and quoted, unlike the version this replaces: bare [ $to_tidy ] is a
-# syntax error the moment there is more than one file, so the hook quietly
-# stopped tidying in exactly the commits that needed it most.
-if [ -n "$to_tidy" ]
+# For the tidy pass, which does not care which list a file came from.  The
+# emptiness tests below concatenate the two without a space instead: with one,
+# "no perl staged at all" would be a string holding a space, which is not empty.
+all_files="$lib_files $test_files"
+
+# Quoted here, because a bare [ $all_files ] is a syntax error the moment there
+# is more than one file -- which would stop the hook working in exactly the
+# commits that need it most.  Unquoted below, because there the word splitting is
+# what turns one string into an argument list.
+if [ -z "$lib_files$test_files" ]
+then
+    :
+elif command -v perltidy >/dev/null 2>&1
 then
     echo "Auto-tidying perl changes..."
 
     # -bext='/' makes perltidy delete the backup it would otherwise leave
     # sitting next to the file as <name>.bak.
-    perltidy -b -bext='/' $to_tidy || {
+    perltidy -b -bext='/' $all_files || {
         echo "pre-commit: perltidy failed, leaving the commit alone."
         exit 1
     }
 
-    git update-index --add $to_tidy
+    git update-index --add $all_files
+    echo "Done."
+else
+    echo "pre-commit: perltidy is not on PATH, skipping the tidy pass."
+fi
+
+# After the tidy pass and never before.  The tidy pass rewrites these files and
+# stages what it wrote, so what critic has to judge is the bytes that will land
+# rather than the ones you saved -- and if you ever turn [CodeLayout::
+# RequireTidyCode] back on, running it first would fail the commit over the one
+# thing the hook had already set out to fix by itself.
+#
+# Three branches rather than two conditions and one else.  Staging nothing the
+# profile judges -- a commit of documentation, say -- is not the same thing as
+# having no perlcritic to judge it with, and one else says the second whenever
+# the first is true.
+if [ -z "$lib_files$test_files" ]
+then
+    echo "Nothing staged that the profile judges."
+elif command -v perlcritic >/dev/null 2>&1
+then
+    echo "Running perlcritic..."
+
+    # Both passes run even when the first has already failed, so one commit
+    # shows every objection rather than one list's worth at a time.
+    critic_failed=''
+
+    if [ -n "$lib_files" ]
+    then
+        perlcritic --profile .perlcriticrc $lib_files || critic_failed=1
+    fi
+
+    # The same profile for t/, less one policy.  A test is not a module: nobody
+    # runs perldoc on it, so requiring POD of it asks for a page no reader will
+    # ever open.  Everything else in the profile still judges it, which is more
+    # than a release does -- all_critic_ok() reads lib/ and not t/, so this hook
+    # is the only thing that looks at a test file at all.
+    if [ -n "$test_files" ]
+    then
+        perlcritic --profile .perlcriticrc --exclude Documentation::RequirePod $test_files || critic_failed=1
+    fi
+
+    if [ -n "$critic_failed" ]
+    then
+        echo
+        echo "pre-commit: perlcritic refused the above.  Either fix them, or say"
+        echo "why with a '## no critic (Policy::Name)' and the reason beside it."
+        echo "To commit without these checks at all: git commit --no-verify"
+        exit 1
+    fi
+
     echo "Done."
+else
+    # Skipped rather than fatal, for the same reason the tidy pass is: a machine
+    # without perlcritic can still commit.
+    echo "pre-commit: perlcritic is not on PATH, skipping the critic pass."
 fi
+
+# Last, because the tests read the files that the tidy pass wrote.  Whatever is
+# staged, and not only Perl: a change to a share/ file or a fixture breaks a
+# test as surely as a change to a module does.  -l for lib/.
+#
+# Without the GIT_ variables that git sets for this hook.  It sets
+# GIT_INDEX_FILE, and GIT_DIR too in a worktree, and git obeys them over -C.  So
+# a test that runs git in a repository of its own would add, commit and write
+# config in this one.  In a subshell, because the git commands of this hook need
+# them: under git commit -a, GIT_INDEX_FILE names the index being committed.
+run_tests() {
+    (
+        unset $(env | sed -n 's/^\(GIT_[A-Za-z0-9_]*\)=.*/\1/p')
+        prove -lm -j8 "$@"
+    ) || {
+        echo "pre-commit: the tests above failed.  To see why, run each failing file"
+        echo "on its own:  prove -lv t/<file>.t"
+        echo "To commit without these checks at all: git commit --no-verify"
+        exit 1
+    }
+}
+
+test -d t || exit 0
+
+# Without tests-covering, every test, because nothing here knows which tests a
+# change can reach.
+if ! command -v tests-covering >/dev/null 2>&1
+then
+    echo "Running every test, because tests-covering is not on PATH; install Perl::Tests::Covering..."
+    run_tests t/
+    exit 0
+fi
+
+# tests-covering chooses the tests that ran a line the diff changes, from the
+# records that git-hooks/post-commit keeps up to date.  It asks
+# .tests-covering-map.pl about each file that no test loads, and the map names
+# the files that no test reads, such as dist.ini.  --unexplained all runs every
+# test for a file that neither can place, such as a fixture.  --full-index,
+# because it matches each file to its records by the blob id on the index line
+# of the diff, and an abbreviated id matches nothing.
+tests=$(git diff --cached --full-index | tests-covering --diff --unexplained all) || exit 1
+if [ -z "$tests" ]
+then
+    echo "No test ran a line that this commit changes."
+    exit 0
+fi
+
+echo "Running the tests that ran what this commit changes..."
+run_tests $tests