[PM-17239] Update Renovate config to configure patch behavior and reassign dependencies #5775
Conversation
|
Great job, no security vulnerabilities found in this Pull Request |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5775 +/- ##
==========================================
- Coverage 47.29% 47.25% -0.05%
==========================================
Files 1647 1648 +1
Lines 74980 75061 +81
Branches 6770 6779 +9
==========================================
+ Hits 35464 35470 +6
- Misses 38033 38107 +74
- Partials 1483 1484 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
.github/renovate.json5
Outdated
"/^Microsoft\\.AspNetCore\\./",
],
matchUpdateTypes: ["patch"],
dependencyDashboardApproval: false
There was a problem hiding this comment.
⛏️ I am noticing inconsistencies with trailing commas on the final line, across all our Renovate configs, and it's bothering me.
There was a problem hiding this comment.
Great catch. I've updated this with 489ec68 (#5775).
I'll check the other configs as well, as now that I see it it's bugging me too.
There was a problem hiding this comment.
For what it's worth, I like the opposite -- no trailing comma 😉.
There was a problem hiding this comment.
Oops.
I can open up another PR with those changes if you'd like. I noted that JSON doesn't support trailing commas but JSON5 does, so for consistency's sake it might make sense to change them all to not have trailing commas regardless of the file type.
There was a problem hiding this comment.
Very late to the party but I will share why I actually like the trailing comma, it all comes down to git merges. Take this file:
{
"prop1": "value",
"prop2": "value"
}
Pretend someone wants to add a new property, your diff will be:
{
"prop1": "value",
- "prop2": "value"
+ "prop2", "value",
+ "prop3": "value",
}
If person 2 made a change to prop1 in parallel, you've now got a merge conflict, but if you had trailing commas, then person 1 only has add a single line and not edit another one.
Tangentially if you wanted to "optimize for git" in another area you would put line breaks between PackageReference's
Lines 25 to 26
in
051f200
so that renovate could make and we could merge 2 PRs with version bumps of packages right next to each other. Because even though the 2 PRs don't edit the exact same line if 2 touching lines are edited git will make you do a merge conflict resolution.
| "/^Microsoft\\.AspNetCore\\./", | ||
| ], | ||
| matchUpdateTypes: ["patch"], | ||
| dependencyDashboardApproval: false |
There was a problem hiding this comment.
⛏️ I am noticing inconsistencies with trailing commas on the final line, across all our Renovate configs, and it's bothering me.
There was a problem hiding this comment.
Great catch. I've updated this with 489ec68 (#5775).
I'll check the other configs as well, as now that I see it it's bugging me too.
There was a problem hiding this comment.
For what it's worth, I like the opposite -- no trailing comma 😉.
There was a problem hiding this comment.
Oops.
I can open up another PR with those changes if you'd like. I noted that JSON doesn't support trailing commas but JSON5 does, so for consistency's sake it might make sense to change them all to not have trailing commas regardless of the file type.
There was a problem hiding this comment.
Very late to the party but I will share why I actually like the trailing comma, it all comes down to git merges. Take this file:
{
"prop1": "value",
"prop2": "value"
}Pretend someone wants to add a new property, your diff will be:
{
"prop1": "value",
- "prop2": "value"
+ "prop2", "value",
+ "prop3": "value",
}If person 2 made a change to prop1 in parallel, you've now got a merge conflict, but if you had trailing commas, then person 1 only has add a single line and not edit another one.
Tangentially if you wanted to "optimize for git" in another area you would put line breaks between PackageReference's
Lines 25 to 26 in 051f200
|
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-17239
📔 Objective
This works in tandem with bitwarden/renovate-config#26, which changes the default behavior for
npmandnuget-managed dependencies to not trigger PRs for patch updates by default.It does the following:
Microsoft.ExtensionsandMicrosoft.AspNetCorepackages.npmandnuget, we are also going to be moving more dependencies into the Platform ownership. This reduces the split of the preconfigured monorepos between teams, thereby eliminating the need for ad-hoc groupings. These have been removed.matchPackagePatternsconfiguration has been deprecated by Renovate, andmatchPackageNamessupports Regex matching for package names (see documentation. As such, I migrated the remaining ad-hoc grouping to use this pattern.⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes