From 029f2f765e7936e79f66e7ed0896ae0841b3ca9d Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Mon, 23 Nov 2020 13:37:44 -0500
Subject: [PATCH] checkPlugins: Revise `npmpublish.yml`

  * Add commentary explaining why things are done the way they are.
  * Delete steps that were added for debugging.
  * Pass `--no-save` when installing `ep_etherpad-lite`.
---
 bin/plugins/lib/npmpublish.yml | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/bin/plugins/lib/npmpublish.yml b/bin/plugins/lib/npmpublish.yml
index 75e0c62d1..8d94ce88a 100644
--- a/bin/plugins/lib/npmpublish.yml
+++ b/bin/plugins/lib/npmpublish.yml
@@ -14,22 +14,39 @@ jobs:
   test:
     runs-on: ubuntu-latest
     steps:
+      # Clone ether/etherpad-lite to ../etherpad-lite so that ep_etherpad-lite
+      # can be "installed" in this plugin's node_modules. The checkout v2 action
+      # doesn't support cloning outside of $GITHUB_WORKSPACE (see
+      # https://github.com/actions/checkout/issues/197), so the repo is first
+      # cloned to etherpad-lite then moved to ../etherpad-lite. To avoid
+      # conflicts with this plugin's clone, etherpad-lite must be cloned and
+      # moved out before this plugin's repo is cloned to $GITHUB_WORKSPACE.
       - uses: actions/checkout@v2
         with:
           repository: ether/etherpad-lite
           path: etherpad-lite
-      # Work around https://github.com/actions/checkout/issues/197:
       - run: mv etherpad-lite ..
+      # etherpad-lite has been moved outside of $GITHUB_WORKSPACE, so it is now
+      # safe to clone this plugin's repo to $GITHUB_WORKSPACE.
       - uses: actions/checkout@v2
       - uses: actions/setup-node@v1
         with:
           node-version: 12
+      # All of ep_etherpad-lite's devDependencies are installed because the
+      # plugin might do `require('ep_etherpad-lite/node_modules/${devDep}')`.
+      # Eventually it would be nice to create an ESLint plugin that prohibits
+      # Etherpad plugins from piggybacking off of ep_etherpad-lite's
+      # devDependencies. If we had that, we could change this line to only
+      # install production dependencies.
       - run: cd ../etherpad-lite/src && npm ci
       - run: npm ci
-      - run: npm install ep_etherpad-lite@file:../etherpad-lite/src
-      - run: git diff
-      - run: ls -la node_modules
-      - run: ls -la node_modules/ep_etherpad-lite/node_modules
+      # This runs some sanity checks and creates a symlink at
+      # node_modules/ep_etherpad-lite that points to ../../etherpad-lite/src.
+      # This step must be done after `npm ci` installs the plugin's dependencies
+      # because npm "helpfully" cleans up such symlinks. :( Installing
+      # ep_etherpad-lite in the plugin's node_modules prevents lint errors and
+      # unit test failures if the plugin does `require('ep_etherpad-lite/foo')`.
+      - run: npm install --no-save ep_etherpad-lite@file:../etherpad-lite/src
       - run: npm test
       - run: npm run lint