Skip to content

Conversation

martincostello
Copy link
Contributor

@martincostello martincostello commented Sep 16, 2025

Resolves #766


Before the change?

  • Change StringEnum<T> back to a sealed record to mitigate the performance regression reported in [BUG]: Performance regression from 3.1.0 #766. My guess would be that readonly struct copy-by-value to happen when values are passed around, where as before they would be passed by reference.

After the change?

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Technically, this is a breaking change from a non-reference type to a reference type (but puts things back as they were before 3.0.0, so you could argue it isn't as 3.1.0 wasn't).

  • Yes
  • No

Change `StringEnum<T>` back to a sealed record to see if it resolves the performance regression reported in octokit#766.
Copy link

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

martincostello added a commit to martincostello/costellobot that referenced this pull request Sep 16, 2025
Consume changes from octokit/webhooks.net#767 to run benchmarks.
@martincostello martincostello marked this pull request as ready for review September 16, 2025 15:59
@Copilot Copilot AI review requested due to automatic review settings September 16, 2025 15:59
Copy link

@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

Reverts StringEnum<T> from a readonly struct back to a sealed record to address performance regression issues. The change removes custom equality operators and updates the Equals method to handle nullable parameters properly.

  • Changes type declaration from readonly struct to sealed record
  • Removes custom equality operators (== and !=) that are now handled by record semantics
  • Updates Equals method parameter to be nullable and adds null safety checks

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🆕 Triage
Development

Successfully merging this pull request may close these issues.

[BUG]: Performance regression from 3.1.0
1 participant