chore: make newSubmoduleBehavior and noSubmodules the default behavior (#1045)

billiegoose · web-flow · commit 9cd7f3db5ef1 · 2020-02-14T13:41:48.000-05:00
BREAKING CHANGE: The `newSubmoduleBehavior` parameter has been removed and is now the default and only behavior, because it is good. And the `noSubmodules` parameter has been removed and is also the default and only behavior. (This only affects you if you a) liked seeing the console warnings or b) were using `statusMatrix` to traverse submodules for some reason.)
diff --git a/README.md b/README.md
diff --git a/__tests__/test-submodules.js b/__tests__/test-submodules.js
@@ -17,9 +17,7 @@ describe('submodule "support"', () => {
       http,
       dir,
       gitdir,
-      url: `https://${localhost}:8888/test-submodules.git`,
-      noSubmodules: true,
-      newSubmoduleBehavior: true
+      url: `https://${localhost}:8888/test-submodules.git`
     })
     // Test
     expect(await listFiles({ fs, gitdir })).toContain('test.empty')
@@ -32,9 +30,7 @@ describe('submodule "support"', () => {
       http,
       dir,
       gitdir,
-      url: `https://${localhost}:8888/test-submodules.git`,
-      noSubmodules: true,
-      newSubmoduleBehavior: true
+      url: `https://${localhost}:8888/test-submodules.git`
     })
     // Test
     await commit({
@@ -59,18 +55,14 @@ describe('submodule "support"', () => {
       dir,
       gitdir,
       ref: 'no-modules',
-      url: `https://${localhost}:8888/test-submodules.git`,
-      noSubmodules: true,
-      newSubmoduleBehavior: true
+      url: `https://${localhost}:8888/test-submodules.git`
     })
     // Test
     await checkout({
       fs,
       dir,
       gitdir,
-      ref: 'master',
-      noSubmodules: true,
-      newSubmoduleBehavior: true
+      ref: 'master'
     })
     expect(await listFiles({ fs, gitdir })).toContain('test.empty')
   })
@@ -82,9 +74,7 @@ describe('submodule "support"', () => {
       http,
       dir,
       gitdir,
-      url: `https://${localhost}:8888/test-submodules.git`,
-      noSubmodules: true,
-      newSubmoduleBehavior: true
+      url: `https://${localhost}:8888/test-submodules.git`
     })
     // Test
     await checkout({ fs, dir, gitdir, ref: 'no-modules' })
diff --git a/src/api/checkout.js b/src/api/checkout.js
@@ -23,8 +23,6 @@ import { join } from '../utils/join.js'
  * @param {boolean} [args.noUpdateHead] - If true, will update the working directory but won't update HEAD. Defaults to `false` when `ref` is provided, and `true` if `ref` is not provided.
  * @param {boolean} [args.dryRun = false] - If true, simulates a checkout so you can test whether it would succeed.
  * @param {boolean} [args.force = false] - If true, conflicts will be ignored and files will be overwritten regardless of local changes.
- * @param {boolean} [args.noSubmodules = false] - If true, will not print out errors about missing submodules support.
- * @param {boolean} [args.newSubmoduleBehavior = false] - If true, will opt into a newer behavior that improves submodule non-support by at least not accidentally deleting them.
  *
  * @returns {Promise<void>} Resolves successfully when filesystem operations are complete
  *
@@ -72,9 +70,7 @@ export async function checkout ({
   dryRun = false,
   // @ts-ignore
   debug = false,
-  force = false,
-  noSubmodules = false,
-  newSubmoduleBehavior = false
+  force = false
 }) {
   try {
     assertParameter('fs', fs)
@@ -94,9 +90,7 @@ export async function checkout ({
       noUpdateHead,
       dryRun,
       debug,
-      force,
-      noSubmodules,
-      newSubmoduleBehavior
+      force
     })
   } catch (err) {
     err.caller = 'git.checkout'
diff --git a/src/api/clone.js b/src/api/clone.js
@@ -21,8 +21,6 @@ import { join } from '../utils/join.js'
  * @param {string} [args.ref] - Which branch to clone. By default this is the designated "main branch" of the repository.
  * @param {boolean} [args.singleBranch = false] - Instead of the default behavior of fetching all the branches, only fetch a single branch.
  * @param {boolean} [args.noCheckout = false] - If true, clone will only fetch the repo, not check out a branch. Skipping checkout can save a lot of time normally spent writing files to disk.
- * @param {boolean} [args.noSubmodules = false] - If true, clone will not log an error about missing submodule support. TODO: Make this not check out submodules when ther's submodule support
- * @param {boolean} [args.newSubmoduleBehavior = false] - If true, will opt into a newer behavior that improves submodule non-support by at least not accidentally deleting them.
  * @param {boolean} [args.noGitSuffix = false] - If true, clone will not auto-append a `.git` suffix to the `url`. (**AWS CodeCommit needs this option**.)
  * @param {boolean} [args.noTags = false] - By default clone will fetch all tags. `noTags` disables that behavior.
  * @param {string} [args.remote = 'origin'] - What to name the remote that is created.
@@ -73,8 +71,6 @@ export async function clone ({
   relative = false,
   singleBranch = false,
   noCheckout = false,
-  noSubmodules = false,
-  newSubmoduleBehavior = false,
   noTags = false,
   headers = {}
 }) {
@@ -108,8 +104,6 @@ export async function clone ({
       relative,
       singleBranch,
       noCheckout,
-      noSubmodules,
-      newSubmoduleBehavior,
       noTags,
       headers
     })
diff --git a/src/api/pull.js b/src/api/pull.js
@@ -45,8 +45,6 @@ import { normalizeCommitterObject } from '../utils/normalizeCommitterObject.js'
  * @param {number} [args.committer.timestamp] - Set the committer timestamp field. This is an alternative to using `date` using an integer number of seconds since the Unix epoch instead of a JavaScript date object.
  * @param {number} [args.committer.timezoneOffset] - Set the committer timezone offset field. This is the difference, in minutes, from the current timezone to UTC. Default is `(new Date()).getTimezoneOffset()`.
  * @param {string} [args.signingKey] - passed to [commit](commit.md) when creating a merge commit
- * @param {boolean} [args.noSubmodules = false] - If true, will not print out an error about missing submodules support. TODO: Skip checkout out submodules when supported instead.
- * @param {boolean} [args.newSubmoduleBehavior = false] - If true, will opt into a newer behavior that improves submodule non-support by at least not accidentally deleting them.
  *
  * @returns {Promise<void>} Resolves successfully when pull operation completes
  *
@@ -83,9 +81,7 @@ export async function pull ({
   headers = {},
   author: _author,
   committer: _committer,
-  signingKey,
-  noSubmodules = false,
-  newSubmoduleBehavior = false
+  signingKey
 }) {
   try {
     assertParameter('fs', _fs)
@@ -126,9 +122,7 @@ export async function pull ({
       headers,
       author,
       committer,
-      signingKey,
-      noSubmodules,
-      newSubmoduleBehavior
+      signingKey
     })
   } catch (err) {
     err.caller = 'git.pull'
diff --git a/src/api/statusMatrix.js b/src/api/statusMatrix.js
@@ -146,7 +146,6 @@ import { worthWalking } from '../utils/worthWalking.js'
  * @param {string} [args.ref = 'HEAD'] - Optionally specify a different commit to compare against the workdir and stage instead of the HEAD
  * @param {string[]} [args.filepaths = ['.']] - Limit the query to the given files and directories
  * @param {function(string): boolean} [args.filter] - Filter the results to only those whose filepath matches a function.
- * @param {boolean} [args.noSubmodules = false] - If true, will skip over submodules completely
  *
  * @returns {Promise<number[][]>} Resolves with a status matrix, described below.
  */
@@ -156,8 +155,7 @@ export async function statusMatrix ({
   gitdir = join(dir, '.git'),
   ref = 'HEAD',
   filepaths = ['.'],
-  filter,
-  noSubmodules = false
+  filter
 }) {
   try {
     assertParameter('fs', fs)
@@ -194,17 +192,13 @@ export async function statusMatrix ({
         // For now, just bail on directories
         const headType = head && (await head.type())
         if (headType === 'tree' || headType === 'special') return
-        if (noSubmodules) {
-          if (headType === 'commit') return null
-        }
+        if (headType === 'commit') return null
 
         const workdirType = workdir && (await workdir.type())
         if (workdirType === 'tree' || workdirType === 'special') return
 
         const stageType = stage && (await stage.type())
-        if (noSubmodules) {
-          if (stageType === 'commit') return null
-        }
+        if (stageType === 'commit') return null
         if (stageType === 'tree' || stageType === 'special') return
 
         // Figure out the oids, using the staged oid for the working dir oid if the stats match.
diff --git a/src/commands/checkout.js b/src/commands/checkout.js
@@ -27,8 +27,6 @@ import { worthWalking } from '../utils/worthWalking.js'
  * @param {boolean} [args.dryRun]
  * @param {boolean} [args.debug]
  * @param {boolean} [args.force]
- * @param {boolean} [args.noSubmodules]
- * @param {boolean} [args.newSubmoduleBehavior]
  *
  * @returns {Promise<void>} Resolves successfully when filesystem operations are complete
  *
@@ -45,9 +43,7 @@ export async function checkout ({
   noUpdateHead,
   dryRun,
   debug,
-  force,
-  noSubmodules,
-  newSubmoduleBehavior
+  force
 }) {
   // Get tree oid
   let oid
@@ -96,9 +92,7 @@ export async function checkout ({
         gitdir,
         ref,
         force,
-        filepaths,
-        noSubmodules,
-        newSubmoduleBehavior
+        filepaths
       })
     } catch (err) {
       // Throw a more helpful error message for this common mistake.
@@ -296,17 +290,7 @@ export async function checkout ({
   }
 }
 
-async function analyze ({
-  fs,
-  onProgress,
-  dir,
-  gitdir,
-  ref,
-  force,
-  filepaths,
-  noSubmodules,
-  newSubmoduleBehavior
-}) {
+async function analyze ({ fs, onProgress, dir, gitdir, ref, force, filepaths }) {
   let count = 0
   return walk({
     fs,
@@ -356,24 +340,12 @@ async function analyze ({
               ]
             }
             case 'commit': {
-              // gitlinks
-              if (!noSubmodules) {
-                console.log(
-                  new GitError(E.NotImplementedFail, {
-                    thing: 'submodule support'
-                  })
-                )
-              }
-              if (newSubmoduleBehavior) {
-                return [
-                  'mkdir-index',
-                  fullpath,
-                  await commit.oid(),
-                  await commit.mode()
-                ]
-              } else {
-                return
-              }
+              return [
+                'mkdir-index',
+                fullpath,
+                await commit.oid(),
+                await commit.mode()
+              ]
             }
             default: {
               return [
@@ -570,19 +542,12 @@ async function analyze ({
               return ['update-blob-to-tree', fullpath]
             }
             case 'commit-commit': {
-              if (newSubmoduleBehavior) {
-                return [
-                  'mkdir-index',
-                  fullpath,
-                  await commit.oid(),
-                  await commit.mode()
-                ]
-              } else {
-                return [
-                  'error',
-                  `update entry Unhandled type ${await stage.type()}-${await commit.type()}`
-                ]
-              }
+              return [
+                'mkdir-index',
+                fullpath,
+                await commit.oid(),
+                await commit.mode()
+              ]
             }
             default: {
               return [
diff --git a/src/commands/clone.js b/src/commands/clone.js
@@ -21,8 +21,6 @@ import { addRemote } from './addRemote.js'
  * @param {string} args.ref
  * @param {boolean} args.singleBranch
  * @param {boolean} args.noCheckout
- * @param {boolean} args.noSubmodules
- * @param {boolean} args.newSubmoduleBehavior
  * @param {boolean} args.noGitSuffix
  * @param {boolean} args.noTags
  * @param {string} args.remote
@@ -61,8 +59,6 @@ export async function clone ({
   relative,
   singleBranch,
   noCheckout,
-  noSubmodules,
-  newSubmoduleBehavior,
   noTags,
   headers
 }) {
@@ -109,8 +105,6 @@ export async function clone ({
     gitdir,
     ref,
     remote,
-    noCheckout,
-    noSubmodules,
-    newSubmoduleBehavior
+    noCheckout
   })
 }
diff --git a/src/commands/pull.js b/src/commands/pull.js
@@ -39,8 +39,6 @@ import { E, GitError } from '../models/GitError.js'
  * @param {number} args.committer.timestamp
  * @param {number} args.committer.timezoneOffset
  * @param {string} [args.signingKey] - passed to [commit](commit.md) when creating a merge commit
- * @param {boolean} [args.noSubmodules = false] - If true, will not print out an error about missing submodules support. TODO: Skip checkout out submodules when supported instead.
- * @param {boolean} [args.newSubmoduleBehavior = false] - If true, will opt into a newer behavior that improves submodule non-support by at least not accidentally deleting them.
  *
  * @returns {Promise<void>} Resolves successfully when pull operation completes
  *
@@ -67,9 +65,7 @@ export async function pull ({
   headers,
   author,
   committer,
-  signingKey,
-  noSubmodules,
-  newSubmoduleBehavior
+  signingKey
 }) {
   try {
     // If ref is undefined, use 'HEAD'
@@ -130,8 +126,6 @@ export async function pull ({
       gitdir,
       ref,
       remote,
-      noSubmodules,
-      newSubmoduleBehavior,
       noCheckout: false
     })
   } catch (err) {
