Skip to content
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

Use utility class to contain map of converted classNames #79

Merged

Conversation

aaronlademann-wf
Copy link
Contributor

Motivation

We want to change the global variable being used to keep track of props / state classes that have been converted to the new boilerplate by sibling migrators to a variable within an object that can be passed around by utilities / migrators that need it.

In doing so, we'll

  • eliminate the risk of different areas of the code unintentionally interfering with each other's data (including in tests like this)
  • make it easier to reason about where the data is used
  • decouple code that reads/writes to this map from the consuming code

Changes

  • Create a ClassToMixinConverter class, which contains a convertedClassNames getter to access the private _convertedClassNames field.
  • Create a single instance of this class within main() of boilerplate_upgrade.dart, and pass the instance around to the migrators that make use of it as constructor arguments.

FYA @sydneyjodon-wk @joebingham-wk @greglittlefield-wf

@aviary2-wf
Copy link

Security Insights

No security relevant content was detected by automated scans.

Action Items

  • Review PR for security impact; comment "security review required" if needed or unsure
  • Verify aviary.yaml coverage of security relevant code

Questions or Comments? Reach out on Slack: #support-infosec.

Copy link
Contributor

@joebingham-wk joebingham-wk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome! Wrapping it in a class makes a lot of sense

+1

Copy link
Contributor

@greglittlefield-wf greglittlefield-wf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1


tearDown(() {
propsAndStateClassNamesConvertedToNewBoilerplate = {};
converter.setConvertedClassNames({});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#nit Instead of sharing this converter and having to worry about cleaning up after it, it'd be nice to have a fresh instance each time.

@aaronlademann-wf aaronlademann-wf merged commit 5617889 into CPLAT-9205-new-boilerplate-codemod Feb 6, 2020
@greglittlefield-wf greglittlefield-wf deleted the get-rid-of-global-var branch February 17, 2020 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants