Skip to content

Conversation

@AntoineDoubovetzky
Copy link

@AntoineDoubovetzky AntoineDoubovetzky commented Feb 20, 2023

Summary

This PR is task 74 from #34872:

Move getTypes functions from utils.js to specific Parsers. Right now we have two Parser classes that takes care of the language specific details and two utils files that contains similar logic. We would like to move everything under the Parsers classes for better OOP architecture and to encourage code-reuse.

Changelog

[Internal] [Changed] - Replace getTypes functions with parser specific methods

Test Plan

I tested using Jest and Flow commands.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 20, 2023
@AntoineDoubovetzky AntoineDoubovetzky changed the title Refactor/move get types to parsers [Codegen] Replace getTypes functions with parser specific methods Feb 20, 2023
@analysis-bot
Copy link

analysis-bot commented Feb 20, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,471,648 +0
android hermes armeabi-v7a 7,793,713 +0
android hermes x86 8,947,054 +0
android hermes x86_64 8,804,356 +0
android jsc arm64-v8a 9,105,576 +0
android jsc armeabi-v7a 8,302,469 +0
android jsc x86 9,155,879 +0
android jsc x86_64 9,414,963 +0

Base commit: 05438c3
Branch: main

@cipolleschi
Copy link
Contributor

Thanks for taking the time to run this refactoring!

@cipolleschi cipolleschi changed the title [Codegen] Replace getTypes functions with parser specific methods [Codegen 84] Replace getTypes functions with parser specific methods Feb 21, 2023
@facebook-github-bot
Copy link
Contributor

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@kyawthura-gg
Copy link
Contributor

@AntoineDoubovetzky FYI, this task is 74, not 84 😁

@AntoineDoubovetzky AntoineDoubovetzky changed the title [Codegen 84] Replace getTypes functions with parser specific methods [Codegen 74] Replace getTypes functions with parser specific methods Feb 22, 2023
@AntoineDoubovetzky
Copy link
Author

Oh sorry, I updated the description

@cipolleschi
Copy link
Contributor

@AntoineDoubovetzky could you please rebase and fix the merge conflicts, please? Now that we have a few people working on Codegen, they will be quite common.. 😅

@AntoineDoubovetzky AntoineDoubovetzky force-pushed the refactor/move-getTypes-to-parsers branch from 160f5ba to abe3cad Compare February 27, 2023 13:03
@AntoineDoubovetzky
Copy link
Author

@cipolleschi It's done!

@facebook-github-bot
Copy link
Contributor

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Mar 1, 2023
@facebook-github-bot
Copy link
Contributor

@cipolleschi merged this pull request in f23f7f4.

OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
This PR is task 74 from facebook#34872:
> Move getTypes functions from utils.js to specific Parsers. Right now we have two Parser classes that takes care of the language specific details and two utils files that contains similar logic. We would like to move everything under the Parsers classes for better OOP architecture and to encourage code-reuse.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[Internal] [Changed] - Replace getTypes functions with parser specific methods

Pull Request resolved: facebook#36225

Test Plan: I tested using Jest and Flow commands.

Reviewed By: rshest

Differential Revision: D43453454

Pulled By: cipolleschi

fbshipit-source-id: 0eebcb55e1af3319e2c35bb462980046329a2c09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants