Skip to content

Conversation

BrennanConroy
Copy link
Member

Add a changelog for recent releases
Link to changelog from both npm packages
Add codecheck for CI to fail if a change was made to the npm projects and the changelog wasn't updated

See #50772

@Copilot Copilot AI review requested due to automatic review settings August 22, 2025 21:13
@BrennanConroy BrennanConroy added the area-signalr Includes: SignalR clients and servers label Aug 22, 2025
@BrennanConroy BrennanConroy requested review from halter73, wtgodbe and a team as code owners August 22, 2025 21:13
Copy link
Contributor

@Copilot Copilot AI left a 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 adds a changelog system for the SignalR TypeScript client packages. It creates a comprehensive changelog documenting recent releases and implements automated validation to ensure the changelog stays updated when changes are made to the TypeScript client code.

Key Changes

  • Adds a new changelog file documenting version history from v8.0.0-preview.1 through v10.0.0-preview.1
  • Links to the changelog from both SignalR npm package README files
  • Implements CI validation to enforce changelog updates when TypeScript client code changes

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
src/SignalR/clients/ts/CHANGELOG.md New changelog file documenting release history and changes
src/SignalR/clients/ts/signalr/README.md Added changelog link to main SignalR package README
src/SignalR/clients/ts/signalr-protocol-msgpack/README.md Added changelog link to msgpack protocol package README
eng/scripts/CodeCheck.ps1 Added validation logic to enforce changelog updates for TypeScript changes

@@ -1,5 +1,7 @@
JavaScript and TypeScript clients for SignalR for ASP.NET Core and Azure SignalR Service

[Changelog](https://github.com/dotnet/aspnetcore/tree/main/src/SignalR/clients/ts/changelog.md)
Copy link
Preview

Copilot AI Aug 22, 2025

Choose a reason for hiding this comment

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

The changelog link is incorrect. It should point to 'CHANGELOG.md' (uppercase) as that's the actual filename created in the diff, not 'changelog.md' (lowercase).

Suggested change
[Changelog](https://github.com/dotnet/aspnetcore/tree/main/src/SignalR/clients/ts/changelog.md)
[Changelog](https://github.com/dotnet/aspnetcore/tree/main/src/SignalR/clients/ts/CHANGELOG.md)

Copilot uses AI. Check for mistakes.

@@ -1,5 +1,7 @@
MsgPack support for SignalR for ASP.NET Core

[Changelog](https://github.com/dotnet/aspnetcore/tree/main/src/SignalR/clients/ts/changelog.md)
Copy link
Preview

Copilot AI Aug 22, 2025

Choose a reason for hiding this comment

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

The changelog link is incorrect. It should point to 'CHANGELOG.md' (uppercase) as that's the actual filename created in the diff, not 'changelog.md' (lowercase).

Suggested change
[Changelog](https://github.com/dotnet/aspnetcore/tree/main/src/SignalR/clients/ts/changelog.md)
[Changelog](https://github.com/dotnet/aspnetcore/tree/main/src/SignalR/clients/ts/CHANGELOG.md)

Copilot uses AI. Check for mistakes.


# Check for relevant changes to SignalR typescript files
$tsChanges = $changedFilesFromTarget | Where-Object { $_ -like "src/SignalR/clients/ts/*" -and $_ -ne "src/SignalR/clients/ts/changelog.md" }
$changelogChanged = $changedFilesFromTarget -contains "src/SignalR/clients/ts/changelog.md"
Copy link
Preview

Copilot AI Aug 22, 2025

Choose a reason for hiding this comment

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

The changelog filename should be 'CHANGELOG.md' (uppercase) to match the actual file created, not 'changelog.md' (lowercase).

Suggested change
$changelogChanged = $changedFilesFromTarget -contains "src/SignalR/clients/ts/changelog.md"
$tsChanges = $changedFilesFromTarget | Where-Object { $_ -like "src/SignalR/clients/ts/*" -and $_ -ne "src/SignalR/clients/ts/CHANGELOG.md" }
$changelogChanged = $changedFilesFromTarget -contains "src/SignalR/clients/ts/CHANGELOG.md"

Copilot uses AI. Check for mistakes.


# Check for relevant changes to SignalR typescript files
$tsChanges = $changedFilesFromTarget | Where-Object { $_ -like "src/SignalR/clients/ts/*" -and $_ -ne "src/SignalR/clients/ts/changelog.md" }
$changelogChanged = $changedFilesFromTarget -contains "src/SignalR/clients/ts/changelog.md"
Copy link
Preview

Copilot AI Aug 22, 2025

Choose a reason for hiding this comment

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

The changelog filename should be 'CHANGELOG.md' (uppercase) to match the actual file created, not 'changelog.md' (lowercase).

Suggested change
$changelogChanged = $changedFilesFromTarget -contains "src/SignalR/clients/ts/changelog.md"
$tsChanges = $changedFilesFromTarget | Where-Object { $_ -like "src/SignalR/clients/ts/*" -and $_ -ne "src/SignalR/clients/ts/CHANGELOG.md" }
$changelogChanged = $changedFilesFromTarget -contains "src/SignalR/clients/ts/CHANGELOG.md"

Copilot uses AI. Check for mistakes.


if (-not $hasOverride) {
LogError "Changes were made to 'src/SignalR/clients/ts/', but no update to 'changelog.md' was found."
LogError "Either update 'src/SignalR/clients/ts/changelog.md' or include '$signalrChangelogOverrideMarker' in your commit message."
Copy link
Preview

Copilot AI Aug 22, 2025

Choose a reason for hiding this comment

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

The changelog filename in the error message should be 'CHANGELOG.md' (uppercase) to match the actual file created, not 'changelog.md' (lowercase).

Suggested change
LogError "Either update 'src/SignalR/clients/ts/changelog.md' or include '$signalrChangelogOverrideMarker' in your commit message."
LogError "Either update 'src/SignalR/clients/ts/CHANGELOG.md' or include '$signalrChangelogOverrideMarker' in your commit message."

Copilot uses AI. Check for mistakes.

Copy link
Member

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

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

Looks good overall! I agree a little bit with Copilot's comment. Even though the URLs aren't case-sensitive maybe we just normalize to lowercase since the CodeCheck scripts assume lowercase as well?

@@ -213,6 +213,25 @@ try {
LogError $file
}
}

# Check for relevant changes to SignalR typescript files
Copy link
Member Author

Choose a reason for hiding this comment

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

@wtgodbe Could I get your review/approval of the infra changes?

Copy link
Contributor

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no conflicting changes have occurred, please rerun validation before merging. You can do this by leaving an /azp run comment here (requires commit rights), or by simply closing and reopening.

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Sep 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-signalr Includes: SignalR clients and servers pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants