chore(deps): update dependency ws to v6.1.2

[renovate]

This PR contains the following updates:

Package	Type	Update	Change	References
ws	devDependencies	patch	6.1.0 -> 6.1.2	source

---

Release Notes

websockets/ws

v6.1.2

Compare Source

Bug fixes

• Restored compatibility with Node.js < 6.13.0 (26436e0).

v6.1.1

Compare Source

Bug fixes

• Queued messages to send are now discarded if the permessage-deflate is enabled
  and the socket closes prematurely (#​1464, #​1471).

---

Renovate configuration

📅 Schedule: "after 6pm every weekday,before 5am every weekday" in timezone America/Los_Angeles.

🚦 Automerge: Enabled.

♻️ Rebasing: Whenever PR is stale, or if you modify the PR title to begin with "rebase!".

🔕 Ignore: Close this PR and you won't be reminded about this update again.

---

• If you want to rebase/retry this PR, check this box

---

This PR has been generated by Renovate Bot. View repository job log here.

[abernix]

Not yet. See #2006.

[abernix]

@rarkins Is there a way to configure Renovate to not merge a PR if there's a blocking review on it?

As seen above, I've tried to hold off on this PR until I could investigate the problem it created. While the blocking review seems to have prevented it for a while, it seems that when the next patch version of the @types/ws (v6.1.2, when this PR was first for v6.1.1) came through and this PR was updated by Renovate, it no longer observed the review? 🤔 Curious on your thoughts.

[rarkins]

@abernix I haven't thought about it before, or deeply understood GitHub's approach. What I think happens is that Reviews become "stale" once there are new commits, e.g. which could be caused by new dependency versions or even just rebasing.

I guess you don't have a rule about "must need one approving reviewer before merging", right? But maybe we can do like you say and deliberately look for disapproving reviewers and stop automerge in that case, even if it's not part of your branch protection settings.

[rarkins]

BTw although reviews are called "stale", it seems they aren't removed like commit statuses are when commits change. So they are tied to the PR yet the commit is somehow used by GitHub to determine staleness or not.

[abernix]

That's correct, @rarkins, we don't have a rule which requires a certain number of reviews. So Renovate is free to merge PRs when the status checks are all passing (though in this case, it also seems that tests were not passing on last commit before it merged? Strange — I haven't seen that before and I don't see anything in particular in the dashboard log which would have supported that decision.).

we can do like you say and deliberately look for disapproving reviewers

Yeah, I haven't thought about this extensively. In the case of a normal PR, there are of course situations where someone might approve and others might not and the PR might move along anyway, but it seems that dependency updates might be safe to deliberately deny until those are "Dismissed" by someone with appropriate permissions.

What I think happens is that Reviews become "stale" once there are new commits

I believe that's correct. In my experience of reviewing PRs, I've always had to go back and explicitly Approve a PR that I've previously Requested changes on, or at least Dismiss the review (leaving it with no opinion from me one way or the other). The review is never dismissed automatically by the addition of another commit. I think this makes sense since I'd want to confirm that the portions I've identified for changes are appropriately addressed after a new commit was pushed by a human. In the case of a bot, I think that I'd feel the same.