-
Notifications
You must be signed in to change notification settings - Fork 7
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
FED-3248 Update non-defaulted state mixin fields to be optional #298
Conversation
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on Slack: #support-infosec. |
@@ -16,16 +16,16 @@ import 'dart:io'; | |||
|
|||
import 'package:args/args.dart'; | |||
import 'package:codemod/codemod.dart'; | |||
import 'package:over_react_codemod/src/dart3_suggestors/null_safety_prep/class_component_required_default_props.dart'; |
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.
I went ahead and removed this since it will be moved in #297
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.
One question and a #nit
); | ||
}); | ||
|
||
test('patches initialized state in legacy classes', () async { |
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.
#nit "initialized" isn't relevant here (copy-paste from other tests?)
test('patches initialized state in legacy classes', () async { | |
test('patches state fields in legacy classes', () async { |
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.
+1 minus merge conflicts
6b88cb1
# Conflicts: # lib/src/executables/null_safety_migrator_companion.dart
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.
+10
@Workiva/release-management-p
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.
+1 from RM
Motivation
When working on a different codemod, we noticed that
ClassComponentRequiredInitialStateMigrator
migrates defaulted state mixin fields, but there's no codemod to migrate the rest of state mixin fields - we should add one to set all other fields to optional.Changes
StateMixinSuggestor
to make all state mixin fields optional and run it afterClassComponentRequiredInitialStateMigrator
Release Notes
Review
See CONTRIBUTING.md for more details on review types (+1 / QA +1 / +10) and code review process.
Please review:
QA Checklist
dart pub global activate --source path .
dart pub global run over_react_codemod:null_safety_migrator_companion
Merge Checklist
While we perform many automated checks before auto-merging, some manual checks are needed: