lib: log as yes/no whether build dir was created

[DeeDeeG]

Checklist

• npm install && npm test passes
• commit message follows commit guidelines

Description of change

When logging "build" dir needed to be created?, put "Yes" or "No" instead of [path that was created] or undefined .

There is a bit of verbose logging in the package that informs the user whether the build dir needed to be created (as opposed to having already been there from a previous node-gyp configure run).

This bit of logging apparently expected to be given a boolean, but was receiving either a path or undefined based on the result of fs.mkdir(). So it was logging either the path of the build dir that was just created or undefined, somewhat nonsensically if you don't know what's going on behind the scenes.

Now it prints either "Yes" or "No".

Before:

gyp verb build dir "build" dir needed to be created? /Users/[user]/[package-name]/build,
gyp verb build dir "build" dir needed to be created? undefined

After:

gyp verb build dir "build" dir needed to be created? Yes,
gyp verb build dir "build" dir needed to be created? No

Note: The CI failure was a timeout on a single Ubuntu run. All other runs passed. This happens to me seemingly at random/as flakiness. I don't think it is caused by this tiny PR. CI passed for this commit at my fork: https://github.com/DeeDeeG/node-gyp/actions/runs/739328726 I believe it would pass here if re-run.

[imatlopez]

I think the path is more helpful? undefined might be ugly but is it worth fixing it for just that case?

[DeeDeeG]

I think the path is more helpful?

Logging the path is very helpful, but it is already logged immediately before, so printing it again is redundant.

gyp verb build dir attempting to create "build" dir: /Users/[user]/[package-name]/build
gyp verb build dir "build" dir needed to be created? /Users/[user]/[package-name]/build

worth fixing [...] ?

I think that's a subjective decision for the maintainers to make. But I think the current logging is confusing, in a way that seems like an accident or oversight. (When printing needed to be created? [path], It seems the log poses a question and then does not answer it. When printing needed to be created? undefined, I as a user have no idea what that means.) It makes the logs more confusing to read and comes across to me as a (relatively trivial) bug.

If maintainers prefer it to print something more-easily understood, like I have done in this PR, they can click a button to merge this. (Okay they also have to figure out the PR-URL: and Reviewed-By: stuff for the commit message. Which I can agree is the kind of tedious thing that might make me think twice about acting on a smaller or less critical PR. It's the little things that can make maintenance a chore...) Anyhow, despite its relative triviality, I felt this was worth at least proposing to maintainers.

P.S. Now that I am thinking explicitly about maintainers' time... I have hidden away the long explanation in the PR body behind a <details> expandable key, and put a much simpler summary there. That should save the maintainers a few minutes of reading! It's a one-line change, plus some formatting whitespace. Reviewing the code would probably take less than half time than reading what I've written here!

[richardlau]

LGTM, given the earlier log.verbose('build dir', 'attempting to create "build" dir: %s', buildDir) a few lines above.

[imatlopez]

ah overlooked that you said it is dupe printing it, you've got a point :)