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

Feedback after our two-week trial #379

Closed
greysteil opened this issue Apr 27, 2018 · 15 comments
Closed

Feedback after our two-week trial #379

greysteil opened this issue Apr 27, 2018 · 15 comments

Comments

@greysteil
Copy link
Contributor

From @StephanBijzitter on November 24, 2017 12:30

Hello!

We've been using Dependabot for almost two weeks now, and it has become time (because it will run out during the weekend) to decide whether we want to continue or not. Whatever our choice will be, it's always good to leave feedback. Here we go!

Greenkeeper

We started using Greenkeeper many months ago and it suited our needs at the time perfectly. It was easy to set up and it simply worked. But then our biggest project broke because of sub-dependencies that had been updated by their authors, and this hit us quite big. We decided to immediately use npm-shrinkwrap, which was the only available solution at the time. This worked, but Greenkeeper did not support it and we only had one repository left to use Greenkeeper.

We figured it was not worth our money to have Greenkeeper only for this repository, which didn't have many dependencies either way and so we stopped using Greenkeeper. Many weeks later we found a plugin for Greenkeeper that would allow npm-shrinkwrap, package-lock and yarn.lock, but it had no support for our CI (Codeship). The pull request that is open to support it (greenkeeperio/greenkeeper-lockfile#72) shows no life and as such we decided to try Dependabot.

Dependabot

Setup

Getting started with Dependabot was as easy as we expected it to be. We had one repository that had a few issues, but that is because of our own workflow. Our CI uses the name of a branch to perform certain operations, and we did not have a case for the Dependabot branches. After adding a regular expression to catch these branches, everything ran fine.

Repositories

Our big repository (that was also mentioned above), could sadly not make use of Dependabot. The PHP updates gave errors on Dependabot's side (which didn't surprise us much) and we're still using that very same, very old npm-shrinkwrap, which is not supported by Dependabot.

We're already in the process of getting this repository phased out by multiple smaller repositories, each focussing on their own domains. Enabling Dependabot for these repositories proved no problem at all. They're all using yarn with yarn.lock, and the pull requests started coming in.

That said, we do have some things we would like to see in Dependabot.

Option for single pull request per update (dependabot/feedback#11)

Some of our repositories are not actively worked on, as they don't require us to work on them. We do like to keep them updated, but having pull requests for each dependency takes quite a bit of time as we'd have to wait for our CI to complete all tests before merging. After merging, the next pull request is rebased and also needs to wait, so this requires us to spend much more time than we'd ideally like to spend.

Support grouping of pull requests (dependabot/feedback#52)

Other repositories are actively used and having pull requests for each dependency is definitely wanted here. However, sometimes dependencies (especially when the major version changes) require other dependencies to also update to the same major version. This is where grouping becomes quite important for us, as currently we have to do that ourselves, manually.

Lerna support (dependabot/feedback#37)

Update: The first steps have been made, see: https://github.com/dependabot/feedback/issues/55#issuecomment-348239012

Though we do not use Lerna, we do have a repository that runs a custom-made Lerna-like flow. The entire release process is very strictly regulated and fully automated. The directory structure is essentially the same as Lerna, so having Lerna support for Dependabot would (likely) allow us to use this repository with Dependabot too!

We are looking into switching that repository to Lerna, but that would require us to fully understand the differences first, and it is currently no priority.

Rebasing and conflicts (dependabot/feedback#38 (comment))

Update: This has now been resolved, see: https://github.com/dependabot/feedback/issues/55#issuecomment-348331696

This is where we had some problems. Rebasing works sometimes, but not always. If it doesn't work, typing @dependabot rebase often did work, but not when there is also a conflict. The conflict had to be resolved manually. We've seen the rebasing issue several times and we only had one conflict during our trial. There seem to be some stability issues here, but it's not a dealbreaker.

Support

I've been in contact with support two times, so I'm going to write this on behalf of myself instead of my team. I don't like to compliment companies/organisations for providing good support, as I believe good support should be expected. However... we use quite a lot of third party services and your support trumps them all. Support is quick and professional, which is very much appreciated. Keep it up!

Conclusion

Although I don't have the final say in this, the person who does have the final say will be reading this. Dependabot is currently a good service, but can be made better with the features mentioned above. Looking through the closed issues on this repository, I can see a lot of interaction with your customers, so I have no doubt you're truly trying to make Dependabot better and better. I'd love to keep using Dependabot.

Copied from original issue: dependabot/feedback#55

@greysteil
Copy link
Contributor Author

This is really helpful feedback - thanks @StephanBijzitter.

FYI, I've spent the last week working on #5, which will fix the "grouping of pull requests" issue for Ruby (where the resolution logic tells us when dependencies need to be updated together), which should lay the groundwork for #52 and potentially #11.

I'm also committed to adding Lerna support, once I've had a chance to look into how it works a little more, and am not at all happy about that rebasing bug - I'll take a look into it once work on #5 is finished (should be this weekend).

@greysteil
Copy link
Contributor Author

Also, on those non-rebasing PRs, can you let me know next time you see one, and don't fix it up yourself? I want to dig into what's causing it, but haven't been able to replicate.

@greysteil
Copy link
Contributor Author

From @StephanBijzitter on November 24, 2017 20:8

Will pass it on to my team, I'm going on vacation today. I have a theory in my head, but as I've not been able to test it yet, I didn't want to mention it. Often when I merge a pull request by Dependabot, I hit the delete branch out of habit. Perhaps this can provide an edge case where Dependabot fails to rebase. But again, I have not been able to test this theory to say it with confidence.

@greysteil
Copy link
Contributor Author

FYI, I think I might have found the cause of the rebasing problem. For repos with a large number of open Dependabot PRs, merging one of the PRs can trigger all of the others to rebase at the same time. That meant Dependabot suddenly made a lot of calls to GitHub, and would sometimes hit an abuse detection threshold. If/when that happened, Dependabot would fail to rebase the PRs, and sometimes leave them hanging in an "updating" state, causing subsequent requests for a rebase to fail.

I've fixed the above by ensuring Dependabot doesn't make too many requests to GitHub in quick succession and am going to put an audit on our PR-updater jobs so we know if any leave a PR in a bad state.

Doubly keen to hear if you experience the problem again!

@greysteil
Copy link
Contributor Author

From @StephanBijzitter on November 28, 2017 11:52

That makes sense. I merged one pull request a few hours ago and Dependabot deleted the branch. Out of the six that are still open, one has been rebased. I'll merge another one and see what happens with the remaining five.

@greysteil
Copy link
Contributor Author

Awesome. Ping me the PR number and repo name, either here or on [email protected], and I'll take a look at it from my side if anything's wrong.

@greysteil
Copy link
Contributor Author

Another update on this: Dependabot can now support mono-repo structures. If you've got a mono-repo with many directories, each of which have their own package.json and package-lock.json (or yarn.lock) you can now add the same language multiple times, specifying a different directory each time. Dependabot will treat the updates completely independently, so you don't have to merge an update to all your dependencies at once.

(In future, we'll also add Lerna support, which would allow you to add Dependabot just once and get PRs that update all your packages at once.)

We're actually using this flow on dependabot-core to keep the files in helpers/yarn and helpers/npm up-to-date.

@greysteil
Copy link
Contributor Author

From @StephanBijzitter on November 30, 2017 21:48

Nice! That's definitely a very good step already! We can use that already since it's basically the exact same as we do manually right now.

As for the rebasing issue (for anyone else reading this), this was largely based on a misunderstanding/miscommunication. We thought Dependabot would always rebase automatically when the branch falls behind master, but Dependabot only automatically rebased when there was a conflict instead. However, since two days ago, Dependabot will now automatically rebase its branches when the repository requires branches to be up to date before merging! This is a very good improvement!

https://twitter.com/dependabot/status/935566382787416064

@greysteil
Copy link
Contributor Author

@StephanBijzitter - I've done a bunch of work on PHP support over the last couple of days and it looks like Dependabot is now producing PRs on your main PHP repo. 🎉

Please do let me know if there's anything amiss with those PRs - PHP support in Dependabot is very new, so may well need some fine-tuning.

@greysteil
Copy link
Contributor Author

From @StephanBijzitter on December 7, 2017 14:19

We noticed the pull requests coming in indeed! We expect none of them to be merged within the next few days, because we have some required status checks that currently cannot pass because of the branch names, leaving the tests pending indefinitely. It's an old repository and we're going to have to be very careful changing the flow, but we'll work on it!

@greysteil
Copy link
Contributor Author

From @StephanBijzitter on December 18, 2017 23:43

@greysteil All dependencies in our PHP project have been updated, although using different pull requests to accommodate for the changes we needed to make (so that may skew your metrics a bit).

However in another project I did notice something strange, which I have reported here:
#180

@greysteil
Copy link
Contributor Author

Awesome! I've fixed up that issue, too (thanks for reporting).

@greysteil
Copy link
Contributor Author

I'm going to close this as it's tracked in other issues. Thanks again for the detailed feedback @StephanBijzitter!

@bbugh
Copy link

bbugh commented May 13, 2019

What is #11 correctly expected to link to? #11 doesn't seem to be related to the single pull request per update, and I can't find a task about single PR per update.

@greysteil
Copy link
Contributor Author

Ack, the links got messed up when we ported across. I think it would have been something about grouping, but it was a long time ago...!

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

2 participants