-
Notifications
You must be signed in to change notification settings - Fork 346
Add automatic KB/LCU download URL fetching in UpdateDependencies #1251
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
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.
Pull Request Overview
This PR removes the old UpdateDependencies automation and replaces it with a new, lightweight console tool that updates manifest.versions.json
variables and automatically fetches KB/LCU download URLs using Playwright.
- Removed legacy script-based updaters and dependency on Microsoft.DotNet.VersionTools
- Introduced a new
Program.cs
CLI using System.CommandLine - Added
ManifestVariableContext
for in-place JSON updates andLcuVariableUpdater
for Playwright-driven URL fetching
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
eng/update-dependencies/ScriptRunnerUpdater.cs | Removed legacy PowerShell script runner |
eng/update-dependencies/Program.cs | New CLI entry point for updating manifest variables |
eng/update-dependencies/Microsoft.DotNet.Framework.UpdateDependencies.csproj | Updated to net9.0, enabled implicit usings, added Playwright, bumped System.CommandLine |
eng/update-dependencies/ManifestVariableContext.cs | Added JSON variable context with recursive resolution and update logic |
eng/update-dependencies/LcuVariableUpdater.cs | New class to fetch KB download URLs via Playwright |
eng/update-dependencies/IVariableUpdater.cs | Added interface for variable updaters |
eng/update-dependencies/IVariableContext.cs | Added interface for manifest variable context |
eng/update-dependencies/Options.cs, DependencyUpdater.cs, CustomFileRegexUpdater.cs | Deleted unused legacy updater code |
Microsoft.DotNet.Framework.Docker.sln | Added the new update-dependencies project to the solution |
Comments suppressed due to low confidence (2)
eng/update-dependencies/Program.cs:16
- [nitpick] Argument names should follow identifier conventions and avoid spaces. Rename to something like
new Argument<string>("manifest-file-path")
to improve usability and consistency.
var manifestFileOption = new Argument<string>("manifest file path")
eng/update-dependencies/LcuVariableUpdater.cs:1
- There are no existing tests for
LcuVariableUpdater
orManifestVariableContext
. Adding unit tests (especially mocking Playwright) will help catch parsing and URL-fetching regressions.
// Licensed to the .NET Foundation under one or more agreements.
Co-authored-by: Copilot <[email protected]>
Part of #1076
The old code in the UpdateDependencies project was unused for several years. There was no automation built around it, and I have been updating dates by hand, so I removed that code. We also needed to remove our dependency on Microsoft.DotNet.VersionTools (dotnet/docker-tools#1658), so I went ahead and removed all of that code as well.
In its place I've added a really simple console app for easily updating variables in
manifest.versions.json
. I added automation to fetch KB download URLs using Playwright based on my previous experiment.