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

extracted buildModuleSchema in parsers-common.js #35147

Conversation

dakshbhardwaj
Copy link
Contributor

Summary

This PR is a task from #34872

Extract the buildModuleSchema function (Flow, TypeScript)in the parsers-commons.js function. The two functions are almost identical except for the filter(property =>) at the end of the function, which is different based on the language.

Changelog

[Internal] [Changed] - Extract the function isModuleRegistryCall in parsers-common.js

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 Oct 31, 2022
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,003,350 +0
android hermes armeabi-v7a 6,379,695 +0
android hermes x86 7,416,186 +0
android hermes x86_64 7,280,093 +0
android jsc arm64-v8a 8,868,346 +0
android jsc armeabi-v7a 7,606,652 +0
android jsc x86 8,926,232 +0
android jsc x86_64 9,409,473 +0

Base commit: ad5e3f6
Branch: main

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: ad5e3f6
Branch: main

@pull-bot
Copy link

PR build artifact for 54ba9e5 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@cipolleschi
Copy link
Contributor

Hi @dakshbhardwaj, thank you for taking this.

Just a question: are you sure you added/committed all the changes? I can see the new function in the parsers-common.js, but I can't see it used, nor the same code removed from the other files.
Could you double check that please? 🙏

@pull-bot
Copy link

PR build artifact for 54ba9e5 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@dakshbhardwaj
Copy link
Contributor Author

Hi @dakshbhardwaj, thank you for taking this.

Just a question: are you sure you added/committed all the changes? I can see the new function in the parsers-common.js, but I can't see it used, nor the same code removed from the other files. Could you double check that please? 🙏

Hi @cipolleschi i haven't removed it from the other files because this pr has dependency from the other tasks as some other functions are also needed to be moved to parsers-common.js so i'll push the code once those pr's are merged

@Pranav-yadav
Copy link
Contributor

Pranav-yadav commented Nov 12, 2022

Hey @dakshbhardwaj !

Just heads up, the dependent changes (for this PR) are merged :)

@cipolleschi
Copy link
Contributor

@dakshbhardwaj could you please rebase on top of main, please?

@cipolleschi
Copy link
Contributor

gentle ping to @dakshbhardwaj, are you still interested working on this?

@cipolleschi
Copy link
Contributor

Closing this as the author is not responsive.

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. hacktoberfest-accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants