refactor(linter): remove outdated comment#16430
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Tiny change to docs only, so merging without review. |
CodSpeed Performance ReportMerging #16430 will not alter performanceComparing Summary
Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR removes a comment that documents an architectural pattern for optimizing binary size in the linter's fixer module. The comment described avoiding T: Into<Foo> generics in internal methods while using them in public APIs.
Key Changes
- Removed a comment explaining why internal methods should avoid generic
Into<T>parameters for binary size optimization
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // NOTE(@DonIsaac): Internal methods shouldn't use `T: Into<Foo>` generics to optimize binary | ||
| // size. Only use such generics in public APIs. | ||
| fn new_fix(&self, fix: CompositeFix, message: Option<Cow<'static, str>>) -> RuleFix { |
There was a problem hiding this comment.
[nitpick] This comment appears to still be relevant and describes an important architectural pattern used in this file. The pattern is actively followed:
- Public methods like
replace(line 110) andinsert_text_before(line 131) useInto<Cow<'static, str>>generics - These public methods call inner functions (e.g., line 112-126) to "avoid megamorphic bloat"
- Internal/private methods like
new_fix(line 55) andinsert_text_at(line 170) take concrete types withoutInto<T>generics
This comment documents why this pattern exists and could help future maintainers understand the architectural decision. Consider keeping it or moving it to a more prominent location if the placement above new_fix is no longer appropriate.
There was a problem hiding this comment.
Ah maybe you're right. I don't know. Closing.

Pure refactor. Remove a comment which is no longer relevant.