Skip to content

Conversation

LittleLittleCloud
Copy link
Contributor

…o latest pre-release versions

@LittleLittleCloud LittleLittleCloud requested a review from a team as a code owner October 3, 2025 21:27
@LittleLittleCloud LittleLittleCloud enabled auto-merge (squash) October 3, 2025 21:29
Copy link
Member

@drewnoakes drewnoakes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some questions about the utility class. Was it added by accident? Seems unrelated to this PR, so easier to just remove it for now, unless I'm missing something.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why add this without any apparent usages? It's fine to add things like this, but better when they're actually used at the time they're added, so that we can tell it adds value to the code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is required by the newer version of Microsoft.DotNet.HotReload.Client source package.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, HotReloadClients requires that extenion. It's from a source package so the change is not in the git history

    public ValueTask ReportCompilationErrorsInApplicationAsync(ImmutableArray<string> compilationErrors, CancellationToken cancellationToken)
        => browserRefreshServer?.ReportCompilationErrorsInBrowserAsync(compilationErrors, cancellationToken) ?? ValueTask.CompletedTask;
image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also update PolySharp to generate this extension code. For now I'd go with this change since it's simpler.


namespace System.Threading.Tasks;

internal static partial class ValueTaskExtensions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why partial?

@LittleLittleCloud LittleLittleCloud merged commit abeccf3 into main Oct 6, 2025
5 checks passed
@LittleLittleCloud LittleLittleCloud deleted the u/xiaoyun/updateVersion branch October 6, 2025 22:38
@dotnet-policy-service dotnet-policy-service bot added this to the 18.1 milestone Oct 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants