Update bl to v4.0.0, rename a few vars, use template literals.

[XhmikosR]

The thing is that this breaks at least on Windows if you use single quotes for the command. So, I'm not sure if this will cause any breakage upstream.

And being that we don't have CI yet on https://github.com/nodejs/changelog-maker I can't tell for sure...

PS. You need to register for GitHub Actions CI.
PPS. I had to use 8.16.1 because there's a bug in actions/setup-node and installs Node.js 8.10 which lacks npm ci

[XhmikosR]

BTW I also tried using the cross-spawn package here and it also failed with the single quotes, which I'd expect it to escape. But maybe I missed something.

I'll wait until @rvagg you can have a look :)

To see this in action see https://github.com/XhmikosR/gitexec/actions

[rvagg]

You should try to keep the scope of individual commits relatively isolated, it's really hard to review and tbh I'd even prefer to deal with these things as separate PRs. I don't really mind what you've done here, it's just hard to extract the important pieces.

From what I can tell the essence of what you're proposing this:

  const child = spawn('bash', [ '-c', gitcmd ], { env: process.env, cwd: repoPath })
  const child = spawn(gitcmd, { env: process.env, cwd: repoPath, shell: true })

So just switching from a bash -c to a shell:true. That seems entirely reasonable to me.

Would you mind trying to pull this into 3 separate commits? The spawn change in one, the lint, style, and dependency changes in another and the CI in a third? I know it's a bit of extra work but I'd be much more comfortable being able to see these things in isolation and judging them each on their own merit.

[XhmikosR]

NP, this was mostly opened as a preview. I'll split the patches now.

[XhmikosR]

@rvagg done. This PR should be merged last and might need a rebase.

[rvagg]

Found a weird bug in GitHub's push-to-fork functionality, I pushed to your master and it closed this PR, believing that there was no diff to my master I suspect. Anyway, managed to work my way out of it and this is now on master and landed.

Thanks for fixing the single-line if statements in this one btw, nice work all up. Will get a release out but I'm going to bump semver-major because of the new Node >=8 restriction.