chore: add dependabot

[MichaelDeBoey]

I also added Dependabot, so you'll get automatic dependency update PRs

https://github.com/xjamundx/eslint-plugin-promise/blob/development/CHANGELOG.md#500

[welcome]

🙌 Thanks for opening this pull request! You're awesome.

[aladdin-add]

friendly ping @feross

[SomethingNew71]

Any chance this could be merged soon? I would love to get this dependency upgraded in my personal projects. Thanks so much for maintaining this project!

[voxpelli]

Curious: Any specific problem this seeks to solve?

[theoludwig]

Curious: Any specific problem this seeks to solve?

See this issue: #184.
Basically since npm@7, npm fails to install if peerDependencies are not met. @voxpelli

[voxpelli]

Curious: Any specific problem this seeks to solve?

See this issue: #184.
Basically since npm@7, npm fails to install if peerDependencies are not met. @voxpelli

Thanks, I've replied there with a solution to that problem.

As far as I can see this PR isn't needed to solve that, it rather would be a fix for a symptom caused by another problem – that of not installing correct peer dependencies – especially with npm 7 reverting back to being strict about peer dependencies it's more important to ensure the workflow of working with peer dependencies gets solved for those with issues than to quickly update the dependencies of this module.

After all: This module will always lag behind the releases of plugin versions as those modules are maintained independently of this module.

[theoludwig]

I think it is better to use the latest up to date versions of dependencies, peerDependencies included and this PR correctly updated the dependencies.

After all: This module will always lag behind the releases of plugin versions as those modules are maintained independently of this module.

That's why we'll try our best to keep everything up to date.

About your solution, I will directly say my thoughts about it in the issue.

[voxpelli]

My suggestion: Remove the dependabot update and the version range changes for tape and eslintand have this PR focus on solving the perceived problem at hand.

I would propose making a dependabot suggestion as a separate PR, the likelihood that the maintainers want that is far lower than that of the eslint-plugin-promise range bump, so coupling the two helps none of them – just splits the focus.

Regarding the eslint and tape bumps – I would say that you should let the maintainers bump them when they see fit. No gain in doing a PR for that. It can also eg. be debated whether such a bump is actually useful.

[theoludwig]

My suggestion: Remove the dependabot update and the version range changes for tape and eslintand have this PR focus on solving the perceived problem at hand.

I agree, that we could do 2 seperated PRs for both dependabot and this specific problem but I don't think it is necessary as the work is already done, and it is what I think we should do.

I would propose making a dependabot suggestion as a separate PR, the likelihood that the maintainers want that is far lower than that of the eslint-plugin-promise range bump, so coupling the two helps none of them – just splits the focus.

Regarding the eslint and tape bumps – I would say that you should let the maintainers bump them when they see fit. No gain in doing a PR for that. It can also eg. be debated whether such a bump is actually useful.

Why do you think that ? We actually use dependabot for most of the standard org repos.
Also, I don't think there is benefit to not bump eslint and tape as we should keep the versions up to date, to avoid bugs, security vulnerabilities and all that sort of things. No one want to have that many warnings when just doing npm install :

No gain in doing a PR for that.

Could you elaborate a little bit more why ? I actually think it is great to keep packages up to date, and opening a PR for that, it is actually what does dependabot. @voxpelli

[voxpelli]

Also, I don't think there is benefit to not bump eslint and tape as we should keep the versions up to date, to avoid bugs, security vulnerabilities and all that sort of things. No one want to have that many warnings when just doing npm install :

Whenever you do a fresh npm install npm will always install the latest compatible versions of your dependencies as defined in your package.json file.

Whether it says ^7.24.0 or ^7.12.1 doesn't matter, it will install the same version anyhow, hence why one can argue whether its actually useful, because from the perspective of this GitHub repo it affects nothing.

Bumping can be useful if one wants to force ones lock file to be updated, but this project doesn't have a lock file so any updating of dev dependencies in a local lock file should probably be up to the developer who adds such a local lock file.

No gain in doing a PR for that.

Could you elaborate a little bit more why ? I actually think it is great to keep packages up to date, and opening a PR for that, it is actually what does dependabot. @voxpelli

Needless changes creates noise and pollutes the git history and clouds the actual core changes. It can also make it noisy/spammy for those following and/or maintaining a project.

Also, while keeping packages up to is good, one also needs to ensure that especially the new major versions of a dependency are still on the same track as oneself is. If it is going in another direction, then one needs to regroup and make a new plan and maybe swap the dependency or fork it.

[theoludwig]

Whenever you do a fresh npm install npm will always install the latest compatible versions of your dependencies as defined in your package.json file.

Whether it says ^7.24.0 or ^7.12.1 doesn't matter, it will install the same version anyhow, hence why one can argue whether its actually useful, because from the perspective of this GitHub repo it affects nothing.

Hence, I don't see why this is problematic to this PR.

Needless changes creates noise and pollutes the git history and clouds the actual core changes. It can also make it noisy/spammy for those following and/or maintaining a project.

If it improves the project, I don't think it is problematic, we should do as much as needed commits and PRs if it actually improves the project, no need to say : "we will not merge this PR because we already have 428 commits on this repo" 🤣, I really don't understand your point. Is it spammy to actually solves a real issue from user land ? There is already 14 👍 on this PR, and many people want that, so I don't think we can consider this as spam. 😆

I'm not saying, that you should make a commit for every letter you change in the code, even if that could be funny. Or to make useless commits, I'm just saying when it actually makes sense and actually improves the project, this is not a problem. 😄

Also, while keeping packages up to is good, one also needs to ensure that especially the new major versions of a dependency are still on the same track as oneself is. If it is going in another direction, then one needs to regroup and make a new plan and maybe swap the dependency or fork it.

Even though, I think that most of your arguments aren't valid, you made a good point there, indeed we should not update dependencies anyhow and no matter what, we should always be cautions on updates and see if it still fits our needs and don't break anything. 👍
We must be all the more cautions, when it is a major version upgrade, as it is the case in this PR. 👍

Actually eslint-plugin-promise@5.1.0, don't change a lot for the standard rules, so it's ready to go for me, that's why I approved it.
But that's why I requested reviews to 2 more maintainers, to see if it's really wanted and all good, if it still fits our use case.

[jamesst20]

@voxpelli

I will quote yourself

I think this needs a "step to reproduce". I just tried this with Node 16.0.0 and npm 7.10.0:

1. mkdir test-npm7-standard
2. cd test-npm7-standard
3. npm init
4. npm install -D eslint-config-standard
5. Success ✅

Resulting package.json
If I continue and then do:

1. npm install -D eslint-plugin-promise
2. Success ✅ eslint-plugin-promise@4.3.1 was installed

If you just expect all of your users to lack behind versions because you're too afraid to keep walking forward and make more commits (which basically is expected from any supported library) then why don't you just deprecate this library ?

Anyway, I will go ahead and move on to standard as recommended by your own readme.

This module is for advanced users. You probably want to use standard instead :)

[xCykrix]

I would definitely like to see this land. The new dependency resolution will make falling behind all the more annoying to resolve for the end-user. Pushing to stay up to date via dependency-based PRs is definitely a good place to start and be notified when updates are available. It will not be automatically merging, and "noise" that is useful isn't problematic. The use of Pull-Request Continuous Integration will allow ensuring consistency, so that isn't a problem. It can still be reviewed and have a system in place to request additional reviews before merging changes, so that isn't a problem.

As trailing to my comment in the referenced issue to this, we cannot always be expected (or may not even be able to period) to use the peer dependency version range requested by this plugin or use legacy flags to bypass the new stricter resolution. It is at our own risk if we don't use the exact range, but having the ranges at least supported is definitely needed now. Dependabot is a good direction (and I hope to see it more period) in keeping projects up to date to avoid resolution issues going forward. It is going to be more difficult, and I expect to see forks or re-releases of no longer maintained projects to combat resolution issues.

[SomethingNew71]

Any update on merging this in? It's a benign change to versions impacting everyone using this package, and preventing them from updating other peer dependencies within their own projects, which is super frustrating. Telling people to "use a different standard" because the maintainers won't merge a simple version bump is really quite silly.

[voxpelli]

preventing them from updating other peer dependencies within their own projects

Have we seen any examples of this? So far these threads contain no such examples. Which other dependencies doesn't work with eslint-plugin-promise at version 4.x? And are there similar PR:s open for those projects to similarly have them fix their peer dependencies?

[erezrokah]

Hi @voxpelli, not exactly the same use case but we can't update eslint-plugin-promise to v5 until eslint-config-standard allows it via peer dependencies, see netlify/eslint-config-node#160 (comment).

I'd be happy to PR just the peer dependencies change if that will help move this forward.

[theoludwig]

Hi @voxpelli, not exactly the same use case but we can't update eslint-plugin-promise to v5 until eslint-config-standard allows it via peer dependencies, see netlify/eslint-config-node#160 (comment).

I'd be happy to PR just the peer dependencies change if that will help move this forward.

This PR is fine for me, we don't need to do another PR just for the peerDependencies change, also that dependabot is a good idea!
Thanks anyway for your help but this PR is fine as is.

[erezrokah]

Thanks anyway for your help but this PR is fine as is.

Thanks 💯 I'll close #186

[voxpelli]

@divlo The dependabot change is not a fix for eslint-plugin-promise@5.x, so these two PR:s doesn't conflict. This one is broader.

[theoludwig]

Right, I just merged #186.
Thanks for your feedbacks! @voxpelli

This PR is still fine to merge, to add dependabot right ?
I'll wait a second maintainer approval before merging this PR, what do you think ?

[LinusU]

disclaimer: I have only read thru this thread hastily so I might have missed something.

What is the problem that's being solved by Dependabot here? And what would be our strategy for using it?

As we don't have a lock file the latest version of each dependency will be used whenever we do an npm install? Or maybe I'm missing something?

I'm not against adding it, but I would be glad to know that problems it solves ☺️

[theoludwig]

Dependabot is useful to always have up to date dependencies, basically, this whole thread, and peerDependency problem could be avoided thanks to this addition.

As we don't have a lock file the latest version of each dependency will be used whenever we do an npm install? Or maybe I'm missing something?

Not if it is a major version, as we're using ^ character.
Even if we're not using package-lock.json file, it is still useful, and it will still update the dependencies in package.json, we are already using dependabot in most of the standard repo and it works pretty good!

[LinusU]

and peerDependency problem could be avoided thanks to this addition.

How would it have been avoided? 🤔

Does Dependabot bump peerDependencies? Does it also keep the previous version as valid? How does it know what the breaking changes are? It seems like a human would have to be involved in a major version bump (since that is breaking) or do the team behind it review the changes before Dependabot send a PR for it?

[...] it works pretty good!

Unless it's bumping breaking changes with some kind of codemod or similar, I still don't understand quite what it does since the latest version (within breaking changes) of every dependency will still be installed whenever you run npm install (since we don't have a lock file)

[theoludwig]

Does Dependabot bump peerDependencies?

I'm pretty sure it does, but I don't want to say things I'm not 100% sure, so yeah maybe it doesn't but even if it is the case, it is still useful for dependencies and devDependencies.

It seems like a human would have to be involved in a major version bump (since that is breaking) or do the team behind it review the changes before Dependabot send a PR for it?

Whenever there is a dependency update, Dependabot will open a PR to update this dependency, that will trigger the CI, and we could directly see if there is something wrong with this dependency update, if there is indeed something wrong we can commit things to the dependabot branch according to the dependency update, if everything is green, it should be safe to merge the dependabot PR.

Also each PR from Dependabot has Release notes, so we can see what changes.

[voxpelli]

It seems like a human would have to be involved in a major version bump.

I would say the same. Since all new major versions can introduce changes that doesn’t align with the direction of Standard or which require releasing a new major of Standard.

This eslint-plugin-promise update was an exception, often it’s more complex.

[SomethingNew71]

[voxpelli]

The change is merged, not yet released

[LinusU]

@SomethingNew71 this should be fixed in version 16.0.3 (which I just released), would you mind trying that again?

[SomethingNew71]

Looking good! 👍🏼

[voxpelli]

Could we simply reuse the one from here https://github.com/standard/standard/blob/master/.github/dependabot.yml

Then get this merged?

I would in a way appreciate a weekly schedule rather than a daily schedule, to reduce the spam in my inbox, but 🤷 We could potentially move to renovatebot.com eventually and use a common shared config for all repos, making it easier to sync such a baseline across all

[theoludwig]

Yes, I think we can basically copy/paste dependabot.yml file from standard and get this PR merged.

If there's an advantage to use renovabot, we could use it, but dependabot work well too.

[LinusU]

Looking at the other repositories, there seems to be a lot of dependabot PRs open in them. I personally feel that this just drowns out other PRs since we don't seem to have a clear strategy on how to handle them.

Do we have any answers to:

• Who is responsible for merging these PRs?
• Should they always just be merged or is there any other manual action required?
• Do we publish new versions after merging this PRs?

I still don't understand what concrete problems adding this solves, so I might sound a bit pessimistic, sorry about that 😅

[voxpelli]

I agree with @LinusU. I just did a PR to another project to fix all the noise it adds fastify/fastify-basic-auth#65

One possibility is to instead add Renovate and configure Renovate to not auto-create any PR:s, but to only keep a dependency dashboard up to date to show what's pending: voxpelli/list-installed#1 Renovate in general is possible to configure to become much less noisy.