Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump ansi-regex from 3.0.0 to 3.0.1 #362

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dependabot[bot]
Copy link

@dependabot dependabot bot commented on behalf of github Jul 30, 2022

Bumps ansi-regex from 3.0.0 to 3.0.1.

Commits

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
  • @dependabot use these labels will set the current labels as the default for future PRs for this repo and language
  • @dependabot use these reviewers will set the current reviewers as the default for future PRs for this repo and language
  • @dependabot use these assignees will set the current assignees as the default for future PRs for this repo and language
  • @dependabot use this milestone will set the current milestone as the default for future PRs for this repo and language

You can disable automated security fix PRs for this repo from the Security Alerts page.

@dependabot dependabot bot added the dependencies Pull requests that update a dependency file label Jul 30, 2022
@mtrezza
Copy link
Collaborator

mtrezza commented Jul 30, 2022

@ToothlessGear This needs to be rebased on master, but I don't see the button. Could you do that, or elevate my permissions then I'll do it?

@ToothlessGear
Copy link
Owner

@mtrezza I can merge it sure, but you are a full collaborator. Rebase merging is enabled, can you recheck?

@mtrezza
Copy link
Collaborator

mtrezza commented Jul 30, 2022

The thing is that this PR doesn't seem to be based on the latest commit on master. The CI doesn't show the new job titles but the old ones. Usually I'd see a button to rebase and merging should be blocked. But in this case either I don't have the permissions to rebase, or the button is not displayed because the rebase isn't required to merge, which IMO should be set as required in the repo settings. If we allow to merge a PR without rebase, the CI tests are less effective and may fail only after merging.

@ToothlessGear
Copy link
Owner

@dependabot rebase

Bumps [ansi-regex](https://github.com/chalk/ansi-regex) from 3.0.0 to 3.0.1.
- [Release notes](https://github.com/chalk/ansi-regex/releases)
- [Commits](chalk/ansi-regex@v3.0.0...v3.0.1)

---
updated-dependencies:
- dependency-name: ansi-regex
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
@dependabot dependabot bot force-pushed the dependabot/npm_and_yarn/ansi-regex-3.0.1 branch from a740932 to f913f65 Compare July 30, 2022 21:11
@ToothlessGear
Copy link
Owner

@mtrezza I've added the latest update requirements for npm, snyk. For node it's only 16 of the matrix since it's LTS.

@mtrezza
Copy link
Collaborator

mtrezza commented Jul 30, 2022

Yes, but I guess we are still officially supporting Node 12, even though it's out of LTS. If we decide to stop supporting Node 12, we'd have to do a major version increment because it's a breaking change, to follow semver. As for now, we still need to test for Node 12 until we announce that breaking change.

In fact, officially we are still supporting Node >=4:

node-gcm/package.json

Lines 83 to 85 in e034e7d

"engines": {
"node": ">= 4"
},

To clean this up, I think we could:

  1. leave all the Node tests mandatory for now (regardless of their LTS support, I bet there are still deployments that run on even older node versions)
  2. merge the most important existing PRs (any bug fixes, minor improvements, etc)
  3. publish a release (to be graceful to users of Node <16)
  4. make the cut: bump the engines.node in package.json and remove obsolete CI tests to support only from latest LTS node version and publish with a major version increment

Regardless, the original issue I mentioned seems to be solved now, I see the update button in another PR:
image

@mtrezza
Copy link
Collaborator

mtrezza commented Jul 30, 2022

I see you've also disabled all merge options except for "Rebase and merge", I wouldn't do that. I would only enable "Squash and merge" and disable all others. We'll also need that for release-automation. A rebase as part of a merge is rather unpredictable. This btw. has nothing to do with requiring a PR to be up-to-date with the base branch, which is a separate setting that you have already set.

@ToothlessGear
Copy link
Owner

1. leave all the Node tests mandatory for now (regardless of their LTS support, I bet there are still deployments that run on even older node versions)

Done.

I would only enable "Squash and merge" and disable all others.

I'd rather keep all of them. If the commits of the PR are nice to follow, well described and atomic, I'd rather rebase than squash. You have the choice when merging.

@mtrezza
Copy link
Collaborator

mtrezza commented Jul 31, 2022

Once we implement release automation we can only allow "squash and merge", other merge types create a commit history that is not parsable for that purpose.

A "rebase and merge" (which is still enabled) doesn't make much sense in our workflow IMO, because a rebase always needs to happen before we approve a PR as the CI needs to run on it. Otherwise the CI runs tests on something that will not be the end result (pre-rebase) of that "rebase and merge" operation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants