-
Notifications
You must be signed in to change notification settings - Fork 14
Update Renovate config #943
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
Conversation
|
| vulnerabilityAlerts: { | ||
| enabled: false, | ||
| }, | ||
| // https://docs.renovatebot.com/configuration-options/#automergetype - We use branch to not get raced by commits to main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm not sure i understand completely the risk of "races" here.
do I understand correctly that with default PR automergeType:
- renovate runs
- renovate creates some PRs
- CI runs on each PR
- renovate runs again (on what schedule exactly?)
- renovate merges PR(s? or only one because then it needs to rebase the rest?) if they are green
but with branch merge type
- renovate runs
- CI runs on each branch
- renovate somehow tracks if tests passed without running again?
- renovate merges branch (but still has to rebase and repeat for all the others?)
we definitely need to consider setting up the schedules so it can actually get a week's work done within the week.
n.b. we would also need to reconfigure CI AND branch protection rules for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You kind of understand them, right. They both rebase I believe but renovate waits for CI to stabilise essentially before opening a PR when using the branch merge type, and crucially, it will not open an PR for automerge things. So it reduces the noise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- As far as I could observe rebases happen based on webhooks and therefor immdiately.
- CI is observed via webhooks as well it seems
- PR and branch behaviour is the same apart from it does not first create PR when automerging was successful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Setup branch protections to support it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using branch for automergeType looks currently not that convincing to me. PR noise mentioned here, however not really seems that an issue, I would prefer to see changes applied in PRs, no matter if they are done by devs or the bot. I am not sure if I really got an issue with racing to main.
This branch type is not a default, it is already important in my opinion. What if a dependency breaks a test, we devs need to create a PR or not to fix the code/test? Generally automerge itself is already quite a feature, I wonder if we should make things even more implicit. I checked few repos where automerge is used (example matrix.org) see 'pr' used, but some good stats how this feature is used would be good to decide. Generally I think we should have a good objection why we need to use 'branch' instead of default 'pr'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the end I am fine with both solutions. They will work either way. I am just not sure how many more PRs will be created by that which cause notifications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the benefit of having PRs that automerge over branches that automerge to you @maheichyk?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could see all the changes that are done to repository using repository pull requests. I would feel strange if there are basically no PRs, but changes are done in background. I would like to see CI pipeline information (tests, console messages, etc) to be able to check possible issues (not sure if that gets stored somewhere in the case of branches)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, I think we talk about only patch automerge, right. So no really strong objections. However, if a patch update is merged once a week, it would not be spammy in terms of notifications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- see everything in PRs: it is true that you will not have a PR for every change. i am not sure how useful the PR history is to browsing history; personally I think git blame is more useful? you will also be able to browse commit history.
- CI pipeline: is stored for commits, so the commits on the "feature branches" will have a CI status. when the branches get merged using "merge" strategy, you can still find them. if the get merged using "squash" or "rebase", then i think it will be harder to find them. CI runs will still be in https://github.com/nordeck/matrix-widget-toolkit/actions/workflows/ci.yml. regardless, even with using branch here instead of PRs, renovate needs to pass all CI, so for anything it merges, CI will have been green. What would you be interested in looking at?
- patch automerge: yes, for now we are only talking about patch, lockfilemaintenance, pin, and similar automerge. not minor or major updates.
- once a week: the current configuration is to automerge during all 5 weekdays (120h) at any time. renovate is triggered every 4h, so it runs automerges up to 30 times per week. if our understanding in another comment thread is correct, then it detect and merges all available automerges for each one. our "once a week" (monday morning) setting is about all non-auto merges, i.e. PRs opened for maintainers to review by renovate.
|
in case i didn't mention it: we should stop pinning minor versions of mui and consider if we need to pin major versions |
a152a1a to
fb8cab5
Compare
|
https://github.com/nordeck/matrix-widget-toolkit/blob/main/.github/workflows/ci.yml#L26 will probably use newest instead of lts? |
|
Testing was helpful with Here are the branches it creates with respective packages and update types:
Some deps are missing (e.g. it refuses to check github actions in dry run) but this is the overall result. I think some branch names can be improved, and I have to review the general logic. |
HarHarLinks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
something about the addLabels does not work yet
|
Moving the general groups from the bottom to the top of the packageRules yields:
I think this is at least close to what we want. |
Reconfigure PR ResultsThis is an reconfigure PR comment to help you understand and re-configure your renovate bot settings. If this Reconfigure PR were to be merged, we'd expect to see the following outcome: Detected Package Files
Configuration SummaryBased on the default config's presets, Renovate will:
What to ExpectWith your current configuration, Renovate will create 18 Pull Requests: Pin dependencies
Update dependency @reduxjs/toolkit to v2.6.1
Update dependency @rollup/plugin-commonjs to v28.0.6
Update dependency i18next-browser-languagedetector to v8.0.5
Update development-dependencies (patch)
Update patch updates (patch)
Update vite dependencies (patch)
Update all non-major dependencies (minor)
Update dependency i18next-browser-languagedetector to v8.1.0
Update dependency react-router-dom to v7.6.2
Update development-dependencies (minor)Update linters (minor)
Update production-dependencies (minor)
Update dependency @vitejs/plugin-basic-ssl to v2
Update major updates (major)
Update mui dependencies to v7 (major)
Update react monorepo to v19 (major)
Lock file maintenance
🚸 Branch creation will be limited to maximum 2 per hour, so it doesn't swamp any CI resources or overwhelm the project. See docs for |
|
a9ed4a6 now yields: |
Signed-off-by: MTRNord <[email protected]>
Signed-off-by: MTRNord <[email protected]>
…stead of labels to extend rather than replace and set rangeStrategy to replace on engine updates. Also do not extend monorepos to prevent matching issues and reintroduce the labeling groups Signed-off-by: MTRNord <[email protected]>
…n't too restrictive Signed-off-by: MTRNord <[email protected]>
Signed-off-by: MTRNord <[email protected]>
Signed-off-by: MTRNord <[email protected]>
Signed-off-by: MTRNord <[email protected]>
Signed-off-by: MTRNord <[email protected]>
Signed-off-by: MTRNord <[email protected]>
…roup Signed-off-by: MTRNord <[email protected]>
f84151e to
a9ed4a6
Compare
HarHarLinks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- what's the conclusion to #943 (comment)?
- and for this TODO #943 (comment)? there is no comment added about the "clock giver"
- let's put #943 (comment) on the todo later list
- CI needs to be set to LTS #943 (comment) is it possible to do quickly or do we do it in a followup?
Signed-off-by: MTRNord <[email protected]>
Signed-off-by: MTRNord <[email protected]>
Both are activated now and it seems like for now we can just run both. Better save than sorry.
you missed #943 (comment)
Done in d12165f |
Ok, dependabot vuln alerts are on implicitly (be default), provided that the repo and renovate app are configured correctly: https://docs.renovatebot.com/configuration-options/#vulnerabilityalerts
I meant for that info to go into .github/renovate.json5. |
Co-authored-by: Kim Brose <[email protected]> Signed-off-by: Marcel-Nordeck <[email protected]>
Signed-off-by: MTRNord <[email protected]>
Fixed |
…mon dependency Signed-off-by: MTRNord <[email protected]>
Signed-off-by: Kim Brose <[email protected]>
(FYI: the renovate branch name makes it run it on the actual renovate for validation which is a nice thing. It doesnt actually do anything more than that. See https://docs.renovatebot.com/config-validation/#validation-of-renovate-config-change-prs for more info)
TODOs:
✔️ Checklist
Signed-off-byline in the message (more info).