fix: close-pr count included unsuccessful runs

[ghinks]

Changed the result to look at whether the array actually had a length as on testing I found that it always said "1 PRs closed" when closing a failing PR.

During the writing of the documents I created a failing PR from the dependency and the wiby close-pr command was always saying that it was closing it when in fact it was not. The promise all was returning an empty array.

[ghinks]

After having written a unit test to cover this scenario where the PRs raised by wiby fail I do believe that the check

if (closedPR && closedPR.length) {
        closedPrs.push(closedPR)
}

was the correct one as the return will always be an array but can possible be empty which means none of the PRs were closed.

[ljharb]

closedPR?.length > 0?

[dominykas]

So the closeDependencyPR function has two return points:

https://github.com/pkgjs/wiby/blob/b0fb895368a8f545130764bcbcf1825962949ab3/lib/closePR.js#L28

and

https://github.com/pkgjs/wiby/blob/b0fb895368a8f545130764bcbcf1825962949ab3/lib/closePR.js#L48

So yeah, it can be empty, or it can be an array. So we could do what Jordan suggests, or we could return an empty array in the first return point. I'm not too bothered with existing approach as well.

However, this does uncover yet another edge case, where I'm not sure the behavior is 100% correct - we do closedPrs.push(closedPR), however closedPr is already an array - should we not spread it (closedPrs.push(...closedPR))? Or even turn closedPrs into a set, to get a unique list of PRs, because purely theoretically, we could have multiple PRs with the same head (targetting different base branches)?

[ghinks]

I did try the optional chaining grammar as LJ suggested but we support node 12 and this feature was not available until node 14. I'm very happy to change the github actions CI to only test from node 14+ but I"m going to wait on feedback

.