-
-
Notifications
You must be signed in to change notification settings - Fork 0
Suppress warning for old localization API usage in generated codes #37
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 adds pragma directive to suppress old localization API warnings (FLAN0001) in generated source code. The generated code legitimately uses old APIs and should not trigger these warnings.
- Added a constant for the pragma warning suppression directive
- Inserted the pragma directive in both localization and enum source generators to suppress FLAN0001 warnings
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
File | Description |
---|---|
Flow.Launcher.Localization.Shared/Constants.cs | Defines the pragma directive constant for suppressing FLAN0001 warnings |
Flow.Launcher.Localization.SourceGenerators/Localize/LocalizeSourceGenerator.cs | Adds pragma directive to suppress old localization API warnings in generated code |
Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs | Adds pragma directive to suppress old localization API warnings in generated enum code |
📝 WalkthroughWalkthroughA constant string for suppressing a specific compiler warning was added to the shared Changes
Sequence Diagram(s)sequenceDiagram
participant SourceGenerator
participant Constants
participant GeneratedSource
SourceGenerator->>Constants: Retrieve SuppressWarning constant
SourceGenerator->>GeneratedSource: Write namespace declaration
SourceGenerator->>GeneratedSource: Insert SuppressWarning directive
SourceGenerator->>GeneratedSource: Continue generating class code
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
♻️ Duplicate comments (1)
Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs (1)
196-199
: Same restore-pragma consideration as inLocalizeSourceGenerator
.To confine suppression strictly to the generated enum helpers, append a corresponding
#pragma warning restore FLAN0001
near the end ofGenerateSource
. Aligning both generators prevents silent suppression leaks if more content is appended later.
🧹 Nitpick comments (2)
Flow.Launcher.Localization.Shared/Constants.cs (1)
31-31
: Constant addition looks good; consider explicit naming.Adding a single-source constant for the pragma directive avoids magic strings in the generators—nice.
Minor nit: a slightly more explicit name such asPragmaSuppressOldLocalizationWarning
would better convey purpose if more suppressions are added later, but this is non-blocking.Flow.Launcher.Localization.SourceGenerators/Localize/LocalizeSourceGenerator.cs (1)
476-479
: Suppress directive has no matching#pragma warning restore
.Inserting the disable pragma early guarantees the generator compiles warning-free, but the suppression now extends to the entire generated file.
For tighter scope and future-proofing (e.g., if additional code is appended that shouldn’t be exempt), add a matching restore just before the file footer:sourceBuilder.AppendLine(Constants.SuppressWarning); sourceBuilder.AppendLine(); … // existing generated content +// Re-enable the warning after generated content +sourceBuilder.AppendLine("#pragma warning restore FLAN0001");Not critical, yet keeps warning hygiene consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Flow.Launcher.Localization.Shared/Constants.cs
(1 hunks)Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs
(1 hunks)Flow.Launcher.Localization.SourceGenerators/Localize/LocalizeSourceGenerator.cs
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs (1)
Flow.Launcher.Localization.Shared/Constants.cs (1)
Constants
(5-35)
Flow.Launcher.Localization.SourceGenerators/Localize/LocalizeSourceGenerator.cs (1)
Flow.Launcher.Localization.Shared/Constants.cs (1)
Constants
(5-35)
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.
Could you post an example of this warning?
Don't forget the version bump if needed. |
I would like to keep it for the next version |
In generated codes, we should not check old localization API usage warning.