- Notifications
You must be signed in to change notification settings - Fork30k
Comments
Rename noFrequencyBasedMinification to useFrequencyBasedMinification#182684
Rename noFrequencyBasedMinification to useFrequencyBasedMinification#182684jhonathanqz wants to merge 1 commit intoflutter:masterfrom
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.
Code Review
The pull request successfully renamesnoFrequencyBasedMinification touseFrequencyBasedMinification inJsCompilerConfig, addressing a TODO to use positive naming for better readability. The logic is correctly updated to maintain existing behavior, ensuring that the--no-frequency-based-minification flag is passed to the compiler when the property is set tofalse. The CLI flag remains unchanged, preserving the existing user interface. Documentation has been added to the new field, and tests have been updated accordingly. The changes are clean and follow the project's style guidelines, specifically regarding the documentation of public members.
Rename noFrequencyBasedMinification to useFrequencyBasedMinification
This PR renames the parameter
noFrequencyBasedMinificationonJsCompilerConfigtouseFrequencyBasedMinification, addressing the in-repo TODO inpackages/flutter_tools/lib/src/web/compiler_config.dart: "consider renaming this to be 'positive'. Double negatives are confusing." (TODO(kevmoo)).What changes: The parameter is now positive:
useFrequencyBasedMinification(defaulttrue). When set tofalse, the compiler still receives--no-frequency-based-minification. The CLI flag remains--no-frequency-based-minification; only the Dart API and internal naming change.Why: Positive naming improves readability (e.g.
useFrequencyBasedMinification: falseis clearer thannoFrequencyBasedMinification: true). No behavior or CLI contract change.Issues fixed: None. This addresses an in-repo TODO; an issue is not required for this trivial refactor per the template.
If you had to change anything in theflutter/tests repo, include a link to the migration guide as per thebreaking change policy.
N/A — no changes in flutter/tests.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel onDiscord.
Note: The Flutter team is currently trialing the use ofGemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.