-
Notifications
You must be signed in to change notification settings - Fork 171
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 dependent version updates #1064
base: develop
Are you sure you want to change the base?
Conversation
Oooh that's a spicy one I think. My knee-jerk reaction is a strong "heck no". However, I'm happy to have the discussion and I can definitely be convinced. @paulf81 what's the context and motivation for this? |
Thanks for be open to discussion! So here's my pitch points:
|
@rafmudaf that was my original reaction too! But I think @paulf81 has a point---it will only open a PR (which we can always reject if needed). If we find that this is happening too often or is overbearing, we can always remove it again. @paulf81 , do you know where the branches that these PRs are for will live? Will they be on NREL/floris? Or on some other fork somehow? |
By my reading, it looks like PRs will be opened targeting the main branch, rather than the develop branch. Perhaps this is correct---if there is a true dependency conflict, it may need to be resolved in a patch release. Otherwise, it looks like we can add |
Good catch, added |
Or move it from weekly to monthly |
Updating dependency versions can have unintended consequences. When things go well, the consequences are positive - improved performance, less memory consumption, improved integration with the rest of the ecosystem. However, when things go bad, it can be difficult to diagnose and the problems may not show up immediately. This is especially true for lower level libraries like Numpy, attrs, or numexpressions - the ones that do things that look like magic and seem to "just work". For this reason, my approach to software stability is to fix something when it's needed but never before that. @paulf81 @misi9170 What are your thoughts on the process for evaluating whether an updated library version is worth incorporating? I anticipate that the reviews for many of these automated pull requests will be along the lines of "Tests pass, looks good to me", but this approach does not consider the qualitative changes the come with a new library version. Some things that might help us get to middle ground are:
The full options list is at the options reference page. |
hi @rafmudaf, these are good points, thanks for them! I guess my main thinking is these concerns mainly apply to the point we decided to merge a PR. Implementing this change will only create the PR, and we could have a policy of "good to know" and simply close 90% of them according to a conservative approach of only updating failing packages. I think there's a good motivation to be made aware at least of when packages we import have moved a version. It's usually a heads up that conflicts will be coming sooner or later as eventually more and more of the ecosystem makes the jump. Take for example the numpy -> 2 PR. We could fairly claim that there was "no problem" in the sense the FLORIS itself works fine with numpy 1, but as one user commented our back dating was making complicated problems for them:
So my main argument is that since we aren't forced to merge the PRs, at least gaining the information they provide by their creation is useful and keeps us from having to learn these manually or through user-submitted issues. Then going through your other points:
This will probably be case by case. Numpy/pandas being probably merit a big discussion/think, where coloredlogs can be simpler.
Dang
This won't work, FLORIS is configured to automatically accept all minor and patch versions within a major version. The only thing that dependabot can alert us to our major versions.This also implies there won't be very many of these regardless of the weekly setting
No strong feeling on this if you'd like to propose something? |
Add dependabot dependent version updates
This PR introduces automated updating of dependent versions via github's dependabot
Following basic settings from here:
https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuring-dependabot-version-updates
When this file is in github then dependabot should generate PRs we can consider to update dependencies. This should save us from having to do this manually.
Could consider patching this into main as well.