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

Updates dependabot configs #596

Merged
merged 1 commit into from
May 29, 2020
Merged

Updates dependabot configs #596

merged 1 commit into from
May 29, 2020

Conversation

Schwad
Copy link

@Schwad Schwad commented May 19, 2020

Description

Many of our dependencies should require a simple human check and then merge.

However there are some dependencies that we still want to be a manual process as they are quite tricky version to version.

Doing so through Github does not allow us to document these changes, so we are using dependabot configs to allow discussion and documentation of what may be ignored.

This was sparked by an ActiveSupport bump which would have broken ruby 2.4 support.

Signed-off-by: Nick Schwaderer nschwaderer@chef.io

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New content (non-breaking change)
  • Breaking change (a content change which would break existing functionality or processes)

Checklist:

  • I have read the CONTRIBUTING document.

@Schwad Schwad requested a review from a team as a code owner May 19, 2020 11:44
@Schwad Schwad requested a review from james-stocks May 19, 2020 11:45
@Schwad
Copy link
Author

Schwad commented May 19, 2020

This sets out a dependabot config file, configs can be found here: https://dependabot.com/docs/config-file/

After a discussion with stocksy, he mentioned that it would be valuable to document what dependencies we don't want bumped every time dependabot comes around (for various reasons). You can do this via a comment in an issue but then we lose the documentation/knowledge as to why something was ignored or handled differently in the first place.

This file will be that config. NOTE: most of the work was just making sure I had a valid config file, and taking an initial stab at dependencies. Happy to remove/add more of these if needed.

@james-stocks james-stocks requested a review from skpaterson May 19, 2020 13:13
Copy link

@skpaterson skpaterson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Schwad this looks great. Up to now we have been careful about testing cloud resource pack dependency updates prior to updating in train but in theory minor/patch version changes should be fine. The other route to consider would be removing the exceptions and e.g. https://dependabot.com/blog/ignoring-major-version-bumps/ but happy to proceed like this.

@clintoncwolfe clintoncwolfe removed their request for review May 19, 2020 16:12
@tas50
Copy link
Contributor

tas50 commented May 19, 2020

If we turn these all off we're going to lose visibility on new releases. Having these PRs opened is just the start of a conversation that should always involve humans, and allows us to make sure our product works with the latest and greatest cloud and system resources. I'd really much rather we use the dependabot chatbot functionality to skip releases instead.

@james-stocks
Copy link

@tas50 , +1 to figuring out a standard for how/when we ignore dependency updates.

I do prefer having a config file for "standing exceptions" - so they each have a git commit that allows everyone in the community to follow what's happened historically. But these "standing exceptions" should only be used for project-specific exceptional cases. It should not be used for "I don't personally benefit from this update so I don't care" skips.

If we use the chatbot, is there a way for other people in the community to follow along with skips that happened in the past?

For this case (cloud API gems), I was expecting that they would update fairly frequently, but each update requires careful testing against downstream to avoid breaking current (previously certified) content.
@skpaterson commented that minor/patch updates should be fine - so maybe we should go with his suggestion and tell dependabot to just ignore these gems if/when a major release comes up.

For ActiveSupport maybe we don't need config to skip this if the ruby 2.4 buildkite steps will demonstrate that updating this fails.

@Schwad
Copy link
Author

Schwad commented May 20, 2020

I just think it's nice that we have this documented where we can review it. Future ActiveSupport will never be mergeable unless we drop ruby 2.4 support and this gives me a place in the code to make note of that.

If I kill it in the chatbot we lose visibility on what's being ignored.

I am +1 on being as liberal with dependabot as possible, and I am grateful that it has kicked off our discussion about these dependencies here as well.

The primary goal of this PR for me was to create a .dependabot/config.yml file and start a dependency discussion.

I think we have two meaningful options:

  1. (most liberal) delete all of the config file but for ActiveSupport (I feel this really needs to be here)

  2. (more conservative) maybe delete some of the agreed cloud dependencies, but for the ones that remain allow all minor and patches through the door with dependabot

@stevendanna
Copy link
Contributor

Small recommendation if y'all go this route: Anything we are ignoring should have a comment with a link to an issue as well.

@stevendanna
Copy link
Contributor

I don't know if this would help with people's concerns, but a thing that has worked well in a few other projects is that we also make updating deps part of the release cycle. That is, right after a release, we also generate a PR bumping all of the dependencies. At that point we have to "re-up" our decision to keep something held back. This has worked well on projects that are in active development. It becomes problematic once the release cadence for a project slows down though.

@Schwad
Copy link
Author

Schwad commented May 20, 2020

+1 good point @stevendanna - I think the activesupport thing does warrant its own issue and a link. WIll do that now. Thanks for the additional perspective/thoughts!

Many of our dependencies should require a simple human check and then merge.

However there are some dependencies that we still want to be a manual process as they are quite tricky version to version.

Doing so through Github does not allow us to document these changes, so we are using dependabot configs to allow discussion and documentation of what may be ignored.

This was sparked by an ActiveSupport bump which would have broken ruby 2.4 support.

Signed-off-by: Nick Schwaderer <nschwaderer@chef.io>
@Schwad Schwad force-pushed the ns/dependabot_config branch from 5d42e3b to ae5203c Compare May 21, 2020 12:49
@Schwad Schwad added the dependencies Pull requests that update a dependency file label May 21, 2020
@Schwad
Copy link
Author

Schwad commented May 21, 2020

I have updated the PR to:

  1. all < 6.0.0 updates for ActiveSupport
  2. Allow all minor/patch/etc updates on the other gems.

If there are dependencies on there that we think might not be as nefarious I'm happy to hear from @skpaterson etc. and can remove those.

Copy link

@james-stocks james-stocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Nick :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants