Renovate: run tests on branches, group deps and adjust schedule to weekends #556

Merged
pat-s merged 15 commits from refs/pull/556/head into main 2023-11-16 20:45:10 +00:00
pat-s commented 2023-11-06 08:28:57 +00:00 (Migrated from gitea.com)

As title.

As title.
justusbunsi (Migrated from gitea.com) reviewed 2023-11-06 08:29:08 +00:00
justusbunsi commented 2023-11-06 18:48:15 +00:00 (Migrated from gitea.com)

TBH, I don't like the idea of directly pushing untested dependency updates to the main branch. Right now, only PRs are built. Pushes to main are not built. That has to change when going that route. Otherwise, it's fully untested (not even lint or templating tests).

I agreed with automerging the PRs, knowing that there are at least some checks. We still miss proper integration testing and verification of used values of these dependencies. As long as we don't have that and enable pushes to main, I wouldn't put my hand in the fire for broken stuff being accidentally released.

If you are aiming for noise reduction, what about scheduling deps on a weekly basis, instead of everytime Renovate is executed? Given the fact that Renovate updates itself, it runs twice each day, causing multiple merge/pr messages in short time and heavily rebase existing dependency PRs.

TBH, I don't like the idea of directly pushing untested dependency updates to the main branch. Right now, only PRs are built. Pushes to main are not built. That has to change when going that route. Otherwise, it's fully untested (not even lint or templating tests). I agreed with automerging the PRs, knowing that there are at least some checks. We still miss proper integration testing and verification of used values of these dependencies. As long as we don't have that and enable pushes to main, I wouldn't put my hand in the fire for broken stuff being accidentally released. If you are aiming for noise reduction, what about scheduling deps on a weekly basis, instead of everytime Renovate is executed? Given the fact that Renovate updates itself, it runs twice each day, causing multiple merge/pr messages in short time and heavily rebase existing dependency PRs.
pat-s commented 2023-11-06 19:59:56 +00:00 (Migrated from gitea.com)

TBH, I don't like the idea of directly pushing untested dependency updates to the main branch. Right now, only PRs are built. Pushes to main are not built. That has to change when going that route. Otherwise, it's fully untested (not even lint or templating tests).

Tests should definitely run on the branches then as well, fully agree. I did not think of this until now, thanks for the reminder! The main point of this change is noise reduction, I don't think we need PRs (and the mails that come with it) for minor dep updates (we can't judge more than the tests do anyhow).

If you are aiming for noise reduction, what about scheduling deps on a weekly basis, instead of everytime Renovate is executed? Given the fact that Renovate updates itself, it runs twice each day, causing multiple merge/pr messages in short time and heavily rebase existing dependency PRs.

Fully agree, I actually thought we already do so as I did a lot of such weekly-schedule changes in many repos. But apparently not here 😄
Have a look the recent changes and let me know what you think.

In short:

  • Open branches/PRs on the weekend
  • Ideally automerge daily early morning (will then only happen once per week)
  • Run tests also on renovate created branches (using a regex filter)

Renote itself should run more often to rebase/update branches. Only the creation of new PRs should be reduced a bit. We can't really work around the major updates as we don't wanna close them but we need to do some manual testing for them.

> TBH, I don't like the idea of directly pushing untested dependency updates to the main branch. Right now, only PRs are built. Pushes to main are not built. That has to change when going that route. Otherwise, it's fully untested (not even lint or templating tests). Tests should definitely run on the branches then as well, fully agree. I did not think of this until now, thanks for the reminder! The main point of this change is noise reduction, I don't think we need PRs (and the mails that come with it) for minor dep updates (we can't judge more than the tests do anyhow). > If you are aiming for noise reduction, what about scheduling deps on a weekly basis, instead of everytime Renovate is executed? Given the fact that Renovate updates itself, it runs twice each day, causing multiple merge/pr messages in short time and heavily rebase existing dependency PRs. Fully agree, I actually thought we already do so as I did a lot of such weekly-schedule changes in many repos. But apparently not here 😄 Have a look the recent changes and let me know what you think. In short: - Open branches/PRs on the weekend - Ideally automerge daily early morning (will then only happen once per week) - Run tests also on renovate created branches (using a regex filter) Renote itself should run more often to rebase/update branches. Only the creation of new PRs should be reduced a bit. We can't really work around the major updates as we don't wanna close them but we need to do some manual testing for them.
pat-s commented 2023-11-06 20:17:29 +00:00 (Migrated from gitea.com)

Hmm I might have created an issue with the webhook trigger by changing the on: definition. I think it should actually work for all PR events and specific branches this way 🤔

Hmm I might have created an issue with the webhook trigger by changing the `on: ` definition. I think it should actually work for all PR events and specific branches this way 🤔
pat-s commented 2023-11-08 10:35:53 +00:00 (Migrated from gitea.com)

@justusbunsi Do you see why the CI is not triggered anymore in this PR given the changes? Maybe I am overlooking something 🤔

@justusbunsi Do you see why the CI is not triggered anymore in this PR given the changes? Maybe I am overlooking something 🤔
justusbunsi commented 2023-11-11 18:51:56 +00:00 (Migrated from gitea.com)

The main point of this change is noise reduction, I don't think we need PRs (and the mails that come with it) for minor dep updates (we can't judge more than the tests do anyhow).

We haven't implemented #409 yet. That is my main concern on automerges. Whether it be via Pull Request or direct push to the main branch. Right now, this is something we should verify manually. Even for minor updates, as we cannot ensure the dependency cuts semver conform releases. I understand that this is not the desired state. Hence the issue #409.

I don't think we need PRs (and the mails that come with it) .....

If you try to reduce the amount of mails, neither automerge Pull Requests nor directly pushing to the branch will really achieve this with the current Renovate configuration for this repository. To my knowledge everyone of the 41 people watching this repository receive mails for any activity: branch push, PR creation/update/merge, issues, comments, anything.

Reducing this notification amount can be achieved by several actions:

  • unwatching the repository, which is not a good solution for maintainers 😉
  • unsubscibe every Renovate Pull Request, quite a manual effort 😉
  • Use Renovate scheduling as you already mentioned and enabled with schedule:weekends preset
  • Group minor dependencies to one Pull Request instead of multiple. That would reduce those mails by 50%. We have a Chart.lock that gets conflicted when one of the two dependencies is merged. That means the other PR requires a rebase (notification) so that it can be merged (notification).

Grouping minor dependencies, combined with PR scheduling on weekends might be a better solution as long as #409 isn't resolved. Grouping Chart.yaml|Chart.lock dependencies by major/minor might be good in general to eliminate the file conflicts.

> The main point of this change is noise reduction, I don't think we need PRs (and the mails that come with it) for minor dep updates (we can't judge more than the tests do anyhow). We haven't implemented #409 yet. That is my main concern on automerges. Whether it be via Pull Request or direct push to the main branch. Right now, this is something we should verify manually. Even for minor updates, as we cannot ensure the dependency cuts semver conform releases. I understand that this is not the desired state. Hence the issue #409. > I don't think we need PRs (and the mails that come with it) ..... If you try to reduce the amount of mails, neither automerge Pull Requests nor directly pushing to the branch will really achieve this with the current Renovate configuration for this repository. To my knowledge everyone of the 41 people watching this repository receive mails for any activity: branch push, PR creation/update/merge, issues, comments, anything. Reducing this notification amount can be achieved by several actions: - `unwatch`ing the repository, which is not a good solution for maintainers 😉 - `unsubscibe` every Renovate Pull Request, quite a manual effort 😉 - Use Renovate `scheduling` as you already mentioned and enabled with `schedule:weekends` preset - Group minor dependencies to one Pull Request instead of multiple. That would reduce those mails by 50%. We have a `Chart.lock` that gets conflicted when one of the two dependencies is merged. That means the other PR requires a rebase (notification) so that it can be merged (notification). Grouping minor dependencies, combined with PR scheduling on weekends might be a better solution as long as #409 isn't resolved. Grouping `Chart.yaml|Chart.lock` dependencies by major/minor might be good in general to eliminate the file conflicts.
justusbunsi commented 2023-11-11 19:00:53 +00:00 (Migrated from gitea.com)

@justusbunsi Do you see why the CI is not triggered anymore in this PR given the changes? Maybe I am overlooking something 🤔

Since Gitea Actions uses the same syntax as GitHub Actions, I think it's because you defined the on as an array instead of object1.

Instead of doing this...

on:
  - pull_request:
      branches:
        - "*"
  - push:
      branches:
        - "renovate/**"

...this should help:

on:
  pull_request:
    branches:
      - "*"
  push:
    branches:
      - "renovate/**"

Maybe even extend push.branches to main.

> @justusbunsi Do you see why the CI is not triggered anymore in this PR given the changes? Maybe I am overlooking something 🤔 Since Gitea Actions uses the same syntax as GitHub Actions, I think it's because you defined the `on` as an array instead of object[^1]. Instead of doing this... ```yaml on: - pull_request: branches: - "*" - push: branches: - "renovate/**" ``` ...this should help: ```yaml on: pull_request: branches: - "*" push: branches: - "renovate/**" ``` Maybe even extend push.branches to `main`. [^1]: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#using-activity-types-and-filters-with-multiple-events
pat-s commented 2023-11-12 11:08:28 +00:00 (Migrated from gitea.com)

Since Gitea Actions uses the same syntax as GitHub Actions, I think it's because you defined the on as an array instead of object1.

Oh damn, these are the things I don't link so much about YAML ;)
Thanks a lot, not sure if I would have ever found this at this point!

> Since Gitea Actions uses the same syntax as GitHub Actions, I think it's because you defined the on as an array instead of object1. Oh damn, these are the things I <put a not so nice word here> don't link so much about YAML ;) Thanks a lot, not sure if I would have ever found this at this point!
pat-s commented 2023-11-12 11:19:16 +00:00 (Migrated from gitea.com)

We haven't implemented #409 yet. That is my main concern on automerges. Whether it be via Pull Request or direct push to the main branch. Right now, this is something we should verify manually. Even for minor updates, as we cannot ensure the dependency cuts semver conform releases. I understand that this is not the desired state. Hence the issue #409.

Yeah, that one bitnami issue was a bit of a bummer/caveat.
Not so easy to solve #409 though. Feels a bit like we're missing the time to tackle the "large issues" (also/mainly looking at me 👀 😄 ).

If you try to reduce the amount of mails, neither automerge Pull Requests nor directly pushing to the branch will really achieve this with the current Renovate configuration for this repository. To my knowledge everyone of the 41 people watching this repository receive mails for any activity: branch push, PR creation/update/merge, issues, comments, anything.

Yes, that's due to the lacking detail in the Gitea repo watch settings. I thought about this also already.

Group minor dependencies to one Pull Request instead of multiple. That would reduce those mails by 50%. We have a Chart.lock that gets conflicted when one of the two dependencies is merged. That means the other PR requires a rebase (notification) so that it can be merged (notification).

Yeah that might be the only action left to reduce it currently. Though actually grouping too many unrelated deps is not the desired state I'd argue. I would favor splittings deps + automerge with good tests (I assume you too).

Grouping minor dependencies, combined with PR scheduling on weekends might be a better solution as long as #409 isn't resolved. Grouping Chart.yaml|Chart.lock dependencies by major/minor might be good in general to eliminate the file conflicts.

Good conclusion. For now, this might be the best way forward 👍

> We haven't implemented #409 yet. That is my main concern on automerges. Whether it be via Pull Request or direct push to the main branch. Right now, this is something we should verify manually. Even for minor updates, as we cannot ensure the dependency cuts semver conform releases. I understand that this is not the desired state. Hence the issue #409. Yeah, that one bitnami issue was a bit of a bummer/caveat. Not so easy to solve #409 though. Feels a bit like we're missing the time to tackle the "large issues" (also/mainly looking at me 👀 😄 ). > If you try to reduce the amount of mails, neither automerge Pull Requests nor directly pushing to the branch will really achieve this with the current Renovate configuration for this repository. To my knowledge everyone of the 41 people watching this repository receive mails for any activity: branch push, PR creation/update/merge, issues, comments, anything. Yes, that's due to the lacking detail in the Gitea repo watch settings. I thought about this also already. > Group minor dependencies to one Pull Request instead of multiple. That would reduce those mails by 50%. We have a Chart.lock that gets conflicted when one of the two dependencies is merged. That means the other PR requires a rebase (notification) so that it can be merged (notification). Yeah that might be the only action left to reduce it currently. Though actually grouping too many unrelated deps is not the desired state I'd argue. I would favor splittings deps + automerge with good tests (I assume you too). > Grouping minor dependencies, combined with PR scheduling on weekends might be a better solution as long as #409 isn't resolved. Grouping Chart.yaml|Chart.lock dependencies by major/minor might be good in general to eliminate the file conflicts. Good conclusion. For now, this might be the best way forward 👍
pat-s commented 2023-11-12 11:25:37 +00:00 (Migrated from gitea.com)

I've grouped all deps besides the actions ones, i.e. we should get one renovate push per week which combines all of these. In addition there will be the major ones for PG and Redis + the Gitea Action deps.

I've grouped all deps besides the actions ones, i.e. we should get one renovate push per week which combines all of these. In addition there will be the major ones for PG and Redis + the Gitea Action deps.
justusbunsi commented 2023-11-16 20:44:44 +00:00 (Migrated from gitea.com)

Ok to merge from @pat-s via Discord chat.
Let's see if we messed up the config. 😆

Ok to merge from @pat-s via Discord chat. Let's see if we messed up the config. 😆
Sign in to join this conversation.
No description provided.