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

Add dependabot #7154

Closed
wants to merge 3 commits into from
Closed

Add dependabot #7154

wants to merge 3 commits into from

Conversation

litetex
Copy link
Member

@litetex litetex commented Sep 23, 2021

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

This PR introduces Dependabot which will automatically create pull-requests for dependency upgrades.

See also #6945

Note that this is my default configuration for projects.

Fixes the following issue(s)

Due diligence

@litetex litetex added the meta Related to the project but not strictly to code label Sep 23, 2021
@TacoTheDank
Copy link
Member

TacoTheDank commented Sep 23, 2021

I really dislike how Dependabot bloats up repositories with PRs, so I'd rather this not be integrated. Plus, there are times where dependency upgrades require manual changes to make them work again. Dependabot also does not take into consideration new features that could be utilized in new releases.

I would much rather support adding a tool like https://github.com/ben-manes/gradle-versions-plugin/releases. It at least tells you that there's new versions, and it leaves it to you to actually do the update (and implement new features if necessary. A prime example would be my PR here #7150. Dependabot would simply bump the version without consideration for new features like GroupieAdapter that was added in 2.9.0, and would leave you to probably make a new PR for adding this (and now there's two PRs for one dependency upgrade where simply one would've been fine).

Also, sometimes circumstances arise where PRs would not be able to be merged automatically, like ExoPlayer. However, Dependabot does not care, and will create a new PR for EVERY SINGLE new version that is released. It opened a PR for ExoPlayer already? It will literally close its own PR and then make a NEW one for the new version. See https://github.com/fossasia/phimpme-android/pulls?q=is%3Apr+is%3Aopen+label%3Adependencies for just how bad this can get. It makes going through the PR list much harder than it needs to be, imo.

Sorry, I just REALLY don't like Dependabot. 😅 I'm more okay with it being in a project like NewPipeExtractor where it's more manageable, but for a project with lots of dependencies like this one, I think it becomes annoying.

@litetex
Copy link
Member Author

litetex commented Sep 24, 2021

@TacoTheDank
Sorry for the long reply but I have to do some counters here 😄

I would much rather support adding a tool like https://github.com/ben-manes/gradle-versions-plugin/releases. It at least tells you that there's new versions, and it leaves it to you to actually do the update (and implement new features if necessary. A prime example would be my PR here #7150. Dependabot would simply bump the version without consideration for new features like GroupieAdapter that was added in 2.9.0, and would leave you to probably make a new PR for adding this (and now there's two PRs for one dependency upgrade where simply one would've been fine).

My problem with the current system is that there are about 50 dependencies and many of them are not up-to-date and nobody knows that until someone takes a look.

You should ALWAYS keep your dependencies up-to-date if there are no drawbacks like breaking changes. Why?
Minor updates usually have things like security or performance fixes.

Also when you usually don't keep things up-to-date with small, simple updates, you will get one big update - and speaking as a guy who saw big migrations e.g. a nearly 1 Mio. LoC program from Java 8 to 11 in the last months - you don't want to do that.
It breaks your neck when you're in such a state. There are sometimes so many problems at once that you don't know what's the exact reason for them. In extreme cases this can lead to the complete rewriting of the program which is very resource-expensive.

Also if you don't update regularly and there is a constant flow of functionality into the code you will have to sooner or later rewrite your complete program and rethink workarounds that have been done for specific parts.

  • It at least tells you that there's new versions, and it leaves it to you to actually do the update

Nobody is forced to merge this right away. Usually when you get new versions you should check the changes and only then merge them.
However if you have very good automatic test coverage (which NewPipe has not) you can - as a developer - just take a look at the automatic test for the PR and if everything is green just merge it in 😄

  • Dependabot would simply bump the version without consideration for new features like GroupieAdapter that was added in 2.9.0, and would leave you to probably make a new PR for adding this

Sorry but I don't get the problem...
I rather have two smaller pull requests than a big one. Easier to test and merge.
If a dependency update has breaking code changes (which is rare when updating often used libraries - they mostly deprecate things first) then simply update it manually and tell dependabot to ignore the current version.

How would you add that?
Running e.g. a check every midnight using GH actions? Well who will read this stuff, determines if there should be an update now and create one?
It's sounds kind of similar to the NewPipeExtractor CI that runs every night - it's mostly to always red and nobody does anything (like opening issues).


Also, sometimes circumstances arise where PRs would not be able to be merged automatically, like ExoPlayer. However, Dependabot does not care, and will create a new PR for EVERY SINGLE new version that is released. It opened a PR for ExoPlayer already? It will literally close its own PR and then make a NEW one for the new version. See https://github.com/fossasia/phimpme-android/pulls?q=is%3Apr+is%3Aopen+label%3Adependencies for just how bad this can get. It makes going through the PR list much harder than it needs to be, imo.

You know that you can configure dependabot...

  • to only bring up major version updates (which I would not recommend as you can miss e.g. security updates)
  • to ignore certain updates (e.g. a major version, etc)

It will literally close its own PR and then make a NEW one for the new version

I understand this aspect, however don't see certain problems there when you keep the PR "stateless".
Note that this was already mentioned here: dependabot/dependabot-core#2264

There are also alternatives like renovate that don't have that behavior which we could also use as it's pretty similar to dependabot but not that easy to implement.

I'm more okay with it being in a project like NewPipeExtractor where it's more manageable, but for a project with lots of dependencies like this one, I think it becomes annoying

There are also other projects with a similar amount of dependencies that use dependabot successfully, e.g. https://github.com/github/super-linter/tree/master/dependencies

You haven't been in projects that use 150+ (directly added - with transitive dependencies it's around 500) dependencies which doesn't use such automated things or?
Because I am and it's kind of annoying to create annually around 10-50 dependency updates manually (mostly due to security fixes).
I don't want to create PRs for dependency updates. I want to add new functionality (and so usually do the guys that give you the money for it).

Hopefully you can now better judge my point of view 😄

@TacoTheDank
Copy link
Member

TacoTheDank commented Sep 24, 2021

@litetex I had kind of a visceral reaction when initially seeing this PR lmao, sorry.

Don't get me wrong, I'm a SUCKER for keeping dependencies updated (quite a few of my PRs are just that). In simplest terms, my problem with Dependabot is how it tends to create a lot of PRs, which muddies up the PR list. Otherwise, I do think it's a good tool for many projects. 😃

How would you add that?
Running e.g. a check every midnight using GH actions? Well who will read this stuff, determines if there should be an update now and create one?
It's sounds kind of similar to the NewPipeExtractor CI that runs every night - it's mostly to always red and nobody does anything (like opening issues).

Oh, Gradle Versions Plugin is an Android Gradle Plugin, not a GitHub CI thing. It simply informs you of new versions by checking the maven repo where the dependency is hosted. It is run manually with a terminal (/ Git Bash).

Note that this was already mentioned here: dependabot/dependabot-core#2264

That's totally what I mean, yeah. If that issue was solved, I would definitely be more inclined towards adding it.

There are also other projects with a similar amount of dependencies that use dependabot successfully, e.g. https://github.com/github/super-linter/tree/master/dependencies

I'll put forth a counterpoint here: that repo manages their PRs quite well. However, looking at their total number of PRs, almost HALF of them are just PRs from Dependabot. And that's sorta my main problem: I now have to sift through them to find PRs of actual substance (per se). Overall, I believe it can become quite unsightly when the amount of PRs builds up.

You haven't been in projects that use 150+ (directly added - with transitive dependencies it's around 500) dependencies which doesn't use such automated things or?
Because I am and it's kind of annoying to create annually around 10-50 dependency updates manually (mostly due to security fixes).
I don't want to create PRs for dependency updates. I want to add new functionality (and so usually do the guys that give you the money for it).

I'm mostly an Android hobbyist that messes around with Java and Kotlin, and most Android projects really don't have that many direct dependencies. But yes, that is a good point.


Not that this matters, but as long as I'm contributing to this project, you really won't have to worry about outdated dependencies 😉

@litetex
Copy link
Member Author

litetex commented Sep 28, 2021

I added the discussion label here.
I think we have to talk about this with the other devs.

Not that this matters, but as long as I'm contributing to this project, you really won't have to worry about outdated dependencies

That kind of statements concerns me because people tend to go into vaccations or due to some kind of other reason stop contributing to the project. If these are the people who update dependencies then there will be no updates 😆

@litetex litetex added the discussion This needs to be discussed before anything is done label Sep 28, 2021
@TacoTheDank
Copy link
Member

I think we have to talk about this with the other devs.

Agreed 👍

That kind of statements concerns me because people tend to go into vaccations or due to some kind of other reason stop contributing to the project. If these are the people who update dependencies then there will be no updates 😆

Definitely true lol

@litetex
Copy link
Member Author

litetex commented Oct 11, 2021

Closing this due to no/negative feedback.

I will reopen it when Taco is on vacation 😂

@litetex litetex closed this Oct 11, 2021
@litetex litetex deleted the add-dependabot branch March 19, 2022 20:09
@AudricV AudricV mentioned this pull request Sep 17, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This needs to be discussed before anything is done meta Related to the project but not strictly to code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a tool to automatically create dependency updates
2 participants