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

Feature Request: Automerge with delay #2243

Closed
simlu opened this issue Jul 26, 2018 · 27 comments
Closed

Feature Request: Automerge with delay #2243

simlu opened this issue Jul 26, 2018 · 27 comments

Comments

@simlu
Copy link
Contributor

simlu commented Jul 26, 2018

I love the auto merge feature and it is extremely useful for me as I maintain a lot of repos.

However after the recent eslint fiasco, I'm a little hesitant wrt auto merging. Ideally there would be an option to only auto merge prs that are open for longer than x hours/days.

What are your thoughts on this?

@greysteil
Copy link
Contributor

Interesting. I think that's legit, and there are a couple of other potential automerge configurations I want to add, too (e.g., only automerge if compatibility score is above a certain number). Leave it with me.

@simlu
Copy link
Contributor Author

simlu commented Nov 26, 2018

Serious exploit happened again. This feature would have prevented damage.

https://github.com/bitpay/copay/issues/9346
dominictarr/event-stream#116
hugeglass/flatmap-stream#2

I would really, really like to see this implemented.

@zeke
Copy link

zeke commented Dec 25, 2018

automerge if compatibility score is above a certain number

Hi @greysteil 👋 What is "compatibility score"?

@greysteil
Copy link
Contributor

@zeke - https://dependabot.com/compatibility-score/

We calculate them based on the CI of all users doing a given update.

@zeke
Copy link

zeke commented Dec 25, 2018

Wow that is really cool. 😍

@zeke
Copy link

zeke commented Dec 26, 2018

@greysteil is there a public API for those scores?

@greysteil
Copy link
Contributor

Not public but there is an API. What are you thinking of using it for? Happy to make the data available 🙂

@greysteil
Copy link
Contributor

FYI, the current API endpoint (about which I make no guarantees!) is http://api.dependabot.com/badges/compatibility_score. Errors should be descriptive enough for you to add the right parameters, and you'll get JSON back if you ask for it.

@zeke
Copy link

zeke commented Dec 28, 2018

What are you thinking of using it for?

Just curious, really. But maybe something like https://github.com/nice-registry/sourceranks? That would require a lot of requests though. 😛

Thanks for sharing that URL! I promise not to abuse it. ✋

I assembled this URL: https://api.dependabot.com/badges/compatibility_score?dependency-name=express&package-manager=npm_and_yarn&target-version=4.16.4

And i see this error:

Either a previous-version and new-version must be provided, or a version-scheme (and optionally a target-version).

What is a version-scheme and what would an example value look like?

@greysteil
Copy link
Contributor

The only version-scheme currently supported is semver - if you provide that then Dependabot will aggregate the updates its done across all SemVer compatible versions (i.e., all patch and minor updates from a post 1.0.0 version).

@gitfool
Copy link

gitfool commented Feb 4, 2019

@greysteil re auto merge config, it would be nice to have option to avoid merge commits.

@greysteil
Copy link
Contributor

Ah, interesting. That's already an option in your dashboard (under account settings), but perhaps we should add it to config files and make it configurable at a per-repo level...

@gitfool
Copy link

gitfool commented Feb 4, 2019

Oh. I completely missed the account settings. Per repo override would be nice too though. 😉

@gitfool
Copy link

gitfool commented Feb 6, 2019

@greysteil I enabled the account setting but the dependabot commit still doesn't look clean as there seem to be two bot accounts: dependabot-bot authored and dependabot[bot] committed; is that right?

See BoardGameGeek.Dungeon/commits/master commit e8ecd3e.

@greysteil
Copy link
Contributor

greysteil commented Feb 6, 2019

@gitfool - that's the best we can do whilst GitHub don't allow app accounts (i.e., dependabot[bot]) to sign commits.

For others reading this: if you have a question about automerging, please create a separate issue or email support@dependabot.com to avoid spamming those waiting for an update on automerging with delay.

@aaronjensen
Copy link

I don't know if it's better, but another option would be to have a setting that only actually opens the PRs if the release is more than X days old.

I think the somewhat tricky thing about all of this is the scenario where a release, say 2.0 comes out, with a major bug. Then 2.0.1 is released 2 days later. If your delay was set to 3 days, what would be the behavior you'd expect? It'd be bad to merge 2.0 if there was already a 2.0.1 fix out. If you debounced it so that you waited until 2.0.1 was released for 3 days, then, for frequently releasing packages, you may be waiting forever.

@simlu
Copy link
Contributor Author

simlu commented Feb 11, 2019

@aaronjensen Not necessarily a problem if you run npm audit and write tests for your use-cases. This is really a delay to allow time for major security flaws in heavily used packages to be uncovered.

@aaronjensen
Copy link

@simlu Not sure I follow. My example/concern is something that would need to be dealt with in some way regardless of why the delay was happening.

@simlu
Copy link
Contributor Author

simlu commented Feb 11, 2019

@aaronjensen If you run your ci and only allow merge after pass this is not a problem if the major bug is (1) logged in npm audit if its a security flaw (2) or fails your tests

So your point isn't really a concern. That release would just be skipped

@aaronjensen
Copy link

@simlu so you're saying that if your CI runs npm audit, and you assume that within those X days the issue would have been reported there, then that build would fail once that version got merged in. I see what you're saying. In that case, you'd still end up with a bad version merged in, it would just not pass your CI on master. That is, unless it was a delay in actually opening the PR rather than a delay in merging that was configurable.

So, in this scenario, if 2.0.1 were to come out after 2.0, while 2.0 was still "pending", then 2.0 would get pulled/merged after the waiting period and 2.0.1 would have its own waiting period.

I'd be good with that.

@simlu
Copy link
Contributor Author

simlu commented Feb 11, 2019

@aaronjensen Correct, that's what I'm saying.

I guess it depends on your flow/setup. We merge into dev -> stage -> master and on a lot of repos we also run tests against PRs from external forks.

So your bad pr might not make it in at all. Npm audit should always be run when you run tests imo. But it depends on how fast you can react to it and how worried you are about security.

@jrjohnson
Copy link

Would love to prevent automerging anything with a new NPM release user. Those are called out in the UI and I'd love for dependabot to force me to check those PRs manually.

@greysteil
Copy link
Contributor

@jrjohnson that makes sense to me. We have a lot on our plate right now, but I'd like to increase the options for specifying automerge conditions in future.

@stale
Copy link

stale bot commented Oct 23, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs within seven days. Thank you for your contributions.

@simlu
Copy link
Contributor Author

simlu commented Oct 23, 2019

I think there is still a use case here that should be supported. Bump

@jrjohnson
Copy link

This should remain open it is still valid.

@infin8x infin8x transferred this issue from dependabot/feedback Jun 29, 2020
@infin8x
Copy link
Contributor

infin8x commented Jul 2, 2020

We've removed automerge from GitHub-native Dependabot due to precisely the sorts of concerns raised in this thread. If/when we bring back auto-merge, we expect to do so in a way that addresses these concerns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants