feat: allow eslint-plugin-promise 6 #198
Conversation
Expand the peerDependency and add a CI matrix to test it
|
🙌 Thanks for opening this pull request! You're awesome. |
| @@ -50,7 +50,7 @@ | |||
| "eslint": "^7.12.1", | |||
| "eslint-plugin-import": "^2.22.1", | |||
| "eslint-plugin-node": "^11.1.0", | |||
| "eslint-plugin-promise": "^4.2.1 || ^5.0.0" | |||
| "eslint-plugin-promise": "^4.2.1 || ^5.0.0 || ^6.0.0" | |||
There was a problem hiding this comment.
Alternatly, this could just change to >=4.21, keep the testing, and update the devDep
There was a problem hiding this comment.
I like specifying the upper version that complies with the standard rule set, so an upgrade doesn't cause unintentional rule changes
|
Would be helpful with an explanation of why this new major version isn't breaking from our perspective. |
|
The v6 is more about ESLint 8 compat, but I added the CI testing to ensure that there is not conflicts going forward, since you seem to have set v4 as a baseline |
|
Right. Looks like it's the node version that's the breaking change |
|
I can add the Node version matrix as well here if you want |
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| eslint-plugin-promise: |
There was a problem hiding this comment.
I would prefer to have this in something like a compat.yml, to keep the main CI file as lean as possible
There was a problem hiding this comment.
Extra YML files here would be interpreted as malformed workflow files. Might be something for when you go for the org-wide action approach and need a repo specific override file
| @@ -50,7 +50,7 @@ | |||
| "eslint": "^7.12.1", | |||
| "eslint-plugin-import": "^2.22.1", | |||
| "eslint-plugin-node": "^11.1.0", | |||
| "eslint-plugin-promise": "^4.2.1 || ^5.0.0" | |||
| "eslint-plugin-promise": "^4.2.1 || ^5.0.0 || ^6.0.0" | |||
There was a problem hiding this comment.
I like specifying the upper version that complies with the standard rule set, so an upgrade doesn't cause unintentional rule changes
Not necessary I think |
There was a problem hiding this comment.
Hey! 😄
Thanks for your PR, but this will already be fixed by this PR: #193 that hopefully, we will soon merge and release, it is ready, I'm waiting for reviews.
|
@divlo yeah, I'm aware of the other PR, but it seems stalled because of the multiple changes. I opened this to fix one of the issues the other PR was dealing with that should be easier to land |
|
The PR #193 has been landed and released in the prerelease |
Expand the peerDependency and add a CI matrix to test it