hertz-contrib / registry

The service registration & discovery extensions for Hertz
Apache License 2.0
27 stars 39 forks source link

chore: add dependabot for deps version update #36

Closed a631807682 closed 1 year ago

a631807682 commented 1 year ago

What type of PR is this?

use dependabot to update project deps version. refer to https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file

What this PR does / why we need it (English/Chinese):

en: Avoid the hassle of manually updating dependencies zh: 避免手动更新依赖

Which issue(s) this PR fixes:

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

GuangmingLuo commented 1 year ago

我昨天看了下,像 etcd 和 redis 新版本有不兼容升级,这是是不是应该谨慎升级呀

a631807682 commented 1 year ago

我昨天看了下,像 ctcd 和 redis 新版本有不兼容升级,这是是不是应该谨慎升级呀

只是自动提PR,不会自动合并,仍然需要CR,所以我觉得谨慎的可能是CR而不是自动检测。 不兼容应该会使单测失败吧?

GuangmingLuo commented 1 year ago

好的,那我没问题了

welkeyever commented 1 year ago

所有三方依赖的更新都需要严格的review和内部稳定性测试,目前应该没有足够的人力能够review & 保证这个更新不会引入潜在的问题。简单来说一个是ROI不高,二个是风险难以把控。

这个依赖更新我们按需搞吧,目前的策略是人工提交,来合入必要的更新;这一点可能相对其他的开源项目来说会相对保守一些,字节线上是有大量服务依赖 hertz 的,相较于其他激进的开源项目,在稳定性的考量这一个点上,我们会趋向于相对保守一些。

根据 Golang 依赖传递策略,框架指定的 SDK 版本其实并不限制用户自己主动的升级行为。如果有用户愿意去升级某些 SDK,可以自行去做这个事情,同时也能够对 SDK 的版本变化拥有自主的控制和感知。

welkeyever commented 1 year ago

所有三方依赖的更新都需要严格的review和内部稳定性测试,目前应该没有足够的人力能够review & 保证这个更新不会引入潜在的问题。简单来说一个是ROI不高,二个是风险难以把控。

这个依赖更新我们按需搞吧,目前的策略是人工提交,来合入必要的更新;这一点可能相对其他的开源项目来说会相对保守一些,字节线上是有大量服务依赖 hertz 的,相较于其他激进的开源项目,在稳定性的考量这一个点上,我们会趋向于相对保守一些。

根据 Golang 依赖传递策略,框架指定的 SDK 版本其实并不限制用户自己主动的升级行为。如果有用户愿意去升级某些 SDK,可以自行去做这个事情,同时也能够对 SDK 的版本变化拥有自主的控制和感知。

update: 以上是针对 Hertz 主库的逻辑,其他contrib没有这么严格,我重新打开了。

welkeyever commented 1 year ago

@a631807682 Seems that the frequency configuration doesn't seem to be working? Check here.

The noise is too much now...

a631807682 commented 1 year ago

@a631807682 Seems that the frequency configuration doesn't seem to be working? Check here.

The noise is too much now...

The frequency is effective, first checked on the day of the merge, and then on the 1st of each month thereafter. But there is indeed too much noise, including CLAassistant detection and automatic reply, as well as request for updates of similar dependencies caused by detecting each package separately. I will try to avoid some noise in the near future, and then provide suggestions for update or revert the commit.

a631807682 commented 1 year ago

The following makes noise:

  1. Each dependency update generates a pull request. Although dependabot has a lot of discussions about the needs of grouped-updates, there is still no good solution. The compromise is to combine the pull request created by dependabot with a new action, but that doesn't avoid the noise.
  2. CLAassistant does not recognize robots. refer to https://github.com/cla-assistant/cla-assistant#can-i-allow-bot-user-contributions
  3. Every PR status change triggers an automatic reply. There is no way to disable it.

@welkeyever Since Scene 1 makes the most noise, and there's nothing we can do about it, I recommend disabling it.

welkeyever commented 1 year ago

The following makes noise:

  1. Each dependency update generates a pull request. Although dependabot has a lot of discussions about the needs of grouped-updates, there is still no good solution. The compromise is to combine the pull request created by dependabot with a new action, but that doesn't avoid the noise.
  2. CLAassistant does not recognize robots. refer to https://github.com/cla-assistant/cla-assistant#can-i-allow-bot-user-contributions
  3. Every PR status change triggers an automatic reply. There is no way to disable it.

@welkeyever Since Scene 1 makes the most noise, and there's nothing we can do about it, I recommend disabling it.

@a631807682 Agree, maybe re-enable it when we find a better way to use it.

a631807682 commented 8 months ago

I saw a discussion about dependabot in cloudwego from other contributors, so I'll update with the latest news. At present, dependabot already supports grouping PRs and meets our needs. If anyone thinks it is necessary to re-enable, you can also try it.

https://github.com/dependabot/dependabot-core/issues/1190#issuecomment-1693360104 https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#groups