Added multi-extension template based on paranext-core/extensions

[tjcouch-sil]

This multi-extension template has configuration and scripts to help with pulling in many copies of the extension template to make multiple extensions. The template adds extensions into its src folder and builds all of them at once. Each extension folder also still functions as its own independent extension and can run Platform.Bible with just that extension alone.

This is largely derived from paranext-core/extensions and reformatted to work with the extension template (which needed some adjustments of its own - paranext/paranext-extension-template#55). Once this is merged, I will change paranext-core/extensions to be a git subtree based on this repo.

---

This change is 

[irahopkinson]

Wow, thanks for all this good work. Just a few minor things to sort out. Otherwise it .

Reviewed 29 of 29 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @tjcouch-sil)

---

README.md line 137 at r1 (raw file):

</details>

[click me](#manually-create-a-new-extension)

It's not clear to me what this is included for? I think this can be removed since a details pane has its own ability to expand and contract.

Code quote:

[click me](#manually-create-a-new-extension)

---

tsconfig.json line 16 at r1 (raw file):

    "strict": true,
    "forceConsistentCasingInFileNames": true,
    // Paranext requires modules to be CommonJS

BTW change to 'Platform.Bible'. There are probably others that need replacing.

Code quote:

Paranext

---

lib/zip-extensions.ts line 24 at r1 (raw file):

  const zipPromises = extensions.map(async (extension) => {
    try {
      // TODO: MAKE SURE THIS WORKS

BTW does this work? If it does remove the comment.

Code quote:

// TODO: MAKE SURE THIS WORKS

---

webpack/webpack.util.ts line 132 at r1 (raw file):

  return staticFile
    .replace(/<name>/g, extensionInfo.name)
    .replace(/<types>/g, extensionInfo.types || '');

BTW possibly more strictly correct to ?? here since its not a boolean operation needed here.

Code quote:

||

[tjcouch-sil]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @irahopkinson)

---

README.md line 137 at r1 (raw file):

Previously, irahopkinson (Ira Hopkinson) wrote…

It's not clear to me what this is included for? I think this can be removed since a details pane has its own ability to expand and contract.

Oops! I was testing whether linking a collapsed header would work. Unfortunately it doesn't work super well, but it does get close to the right place. Removed.

---

tsconfig.json line 16 at r1 (raw file):

Previously, irahopkinson (Ira Hopkinson) wrote…

BTW change to 'Platform.Bible'. There are probably others that need replacing.

I figured we'd do all this when we did the rebrand, but I went ahead and did it now.

---

lib/zip-extensions.ts line 24 at r1 (raw file):

Previously, irahopkinson (Ira Hopkinson) wrote…

BTW does this work? If it does remove the comment.

Oops! Yes, I just tested that it works. Removed this comment. Thanks for catching this!

---

webpack/webpack.util.ts line 132 at r1 (raw file):

Previously, irahopkinson (Ira Hopkinson) wrote…

BTW possibly more strictly correct to ?? here since its not a boolean operation needed here.

I chose this as opposed to ?? because I thought it would be more fail-safe to return an empty string over various falsy values like 0 and false, but it doesn't really matter; maybe it's more expected functionality this way...? I'll just change it since I'm right here already.

[irahopkinson]

Reviewed 10 of 10 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @tjcouch-sil)

---

tsconfig.json line 16 at r1 (raw file):

Previously, tjcouch-sil (TJ Couch) wrote…

I figured we'd do all this when we did the rebrand, but I went ahead and did it now.

Thanks, I only mention it here because these are all new files so thought it best not to propigate the problem.

[lyonsil]

I'm already following up in Discord, but when I created a repo based on this template and added an extension, I'm seeing errors running npm run build and npm run package.

Separately, can we split scripts that are meant to run on their own from modules that are just meant to export things? Thelib folder seems to have a mix of both.

Reviewed 19 of 29 files at r1, 10 of 10 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @tjcouch-sil)

---

LICENSE line 3 at r2 (raw file):

MIT License

Copyright (c) 2023-present SIL International

Adding "present" is probably not actually the right thing here. I'm not a lawyer, but I haven't seen this before so I did a little searching. https://www.quora.com/If-you-update-a-site-does-the-copyright-year-change

Separately, is SIL really claiming IP ownership here? I thought we explicitly weren't doing that. Even if we are for the core repo, we probably don't want to do that with a template repo that mostly non-SIL orgs will be cloning/forking/templating.

---

lib/add-remotes.ts line 8 at r2 (raw file):

  try {
    await execGitCommand(
      'git remote add paranext-multi-extension-template https://github.com/paranext/paranext-multi-extension-template',

It makes me a bit nervous to see URLs embedded as strings here. We can always do a search and replace later, so this isn't blocking. Having a single place where all repo names and URLs are pulled from might be nicer.

---

lib/git.util.ts line 21 at r2 (raw file):

 * @param command
 * @param options The options for the exec command. Add quiet to not log anything
 */

Just curious if you looked into anything like simple-git. It looks like we end up string parsing output in some scripts that will require maintenance over time as git changes and we move our repo to a new org.

Actually on the string parsing point, the approach taken with these scripts means people with locales set to something other than English won't be able to use this utility properly since the git output will be different. The git commands themselves don't change, but developers who are more comfortable in other languages might have set their output to other languages.

Maybe none of this matters, so I'm leaving it as non-blocking.

---

lib/git.util.ts line 47 at r2 (raw file):

 *
 * @param quiet Whether to log an error if there are working changes
 * @returns 1 if there were working changes, 0 otherwise

As a best practice, it would nice to not return integers with special meanings. Either true/false or an enum/object. Same thought in the next function.

This is non-blocking, but it feels very "C like" to do things like this.

[lyonsil]

Actually, I wonder if it's possible/reasonable to add a test that automatically creates a template from this repo and runs a few scripted commands to make sure that you can add a couple of extensions and the final result builds/packages properly.

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @tjcouch-sil)

[lyonsil]

The build issue has been resolved. I needed to target specific branches in the individual extension template.

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @tjcouch-sil)

[tjcouch-sil]

I lean toward keeping the scripts and util modules (which do have .util.ts on their name) together personally. Feels kinda bloaty to add more folders that most people probably won't look at. I'm open to suggestions, though.

Tests would be smart! But I don't want to write them right now 😬

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @lyonsil)

---

LICENSE line 3 at r2 (raw file):

Previously, lyonsil (Matt Lyons) wrote…

Adding "present" is probably not actually the right thing here. I'm not a lawyer, but I haven't seen this before so I did a little searching. https://www.quora.com/If-you-update-a-site-does-the-copyright-year-change

Separately, is SIL really claiming IP ownership here? I thought we explicitly weren't doing that. Even if we are for the core repo, we probably don't want to do that with a template repo that mostly non-SIL orgs will be cloning/forking/templating.

I agree that it seems weird; I have never seen this written in a copyright before this one to my knowledge. The copyright statement has said that for 11 months (I didn't write it this way), and I was just copying stuff around and making it consistent throughout our repos. I think I'll go change it to 2022-2023 in other places.

We discussed offline that it is probably best to make this LICENSE file generic on the template repos so people don't feel like we're trying to take their code or something.

---

lib/add-remotes.ts line 8 at r2 (raw file):

Previously, lyonsil (Matt Lyons) wrote…

It makes me a bit nervous to see URLs embedded as strings here. We can always do a search and replace later, so this isn't blocking. Having a single place where all repo names and URLs are pulled from might be nicer.

Seems nice! Done

---

lib/git.util.ts line 21 at r2 (raw file):

Previously, lyonsil (Matt Lyons) wrote…

Just curious if you looked into anything like simple-git. It looks like we end up string parsing output in some scripts that will require maintenance over time as git changes and we move our repo to a new org.

Actually on the string parsing point, the approach taken with these scripts means people with locales set to something other than English won't be able to use this utility properly since the git output will be different. The git commands themselves don't change, but developers who are more comfortable in other languages might have set their output to other languages.

Maybe none of this matters, so I'm leaving it as non-blocking.

I looked at npm git libraries, but they didn't seem to support subtrees. I figured splitting between command-line and libraries wasn't worth the hassle.

Regarding string parsing, unfortunately, I don't know what else to do. I looked into the source of simple-git a bit, and I don't see anywhere they're doing filtering like this. I did use --porcelain=v2 on git status to make it machine-readable, but it seems that isn't available on all git commands unfortunately.

Now that I think about it, regarding localization, I suppose I could rework all the error strings into some template format and throw them in git.util.ts or somewhere, but maybe we can just wait and do something once someone runs into a problem...? Looks like maybe here's a list of all its localizations: https://github.com/git/git/tree/master/po eh I just went ahead and did it for now. Kinda dirty, but seems a bit better than before.

---

lib/git.util.ts line 47 at r2 (raw file):

Previously, lyonsil (Matt Lyons) wrote…

As a best practice, it would nice to not return integers with special meanings. Either true/false or an enum/object. Same thought in the next function.

This is non-blocking, but it feels very "C like" to do things like this.

Agreed. This was intended to be able to be returned as an error code. Buuut I don't think any of these scripts actually return an error code right now since it's all async IIFEs. Reworked

[irahopkinson]

Reviewed 7 of 7 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @lyonsil)

[lyonsil]

Thanks for all the updates. Look good!

One day you will learn to love tests! (maybe...)

Reviewed 7 of 7 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @tjcouch-sil)