Skip to content

Reduce visibility of DevInternalSettings class - #37256

Closed
cortinico wants to merge 1 commit into
react:mainfrom
cortinico:export-D45566473
Closed

Reduce visibility of DevInternalSettings class#37256
cortinico wants to merge 1 commit into
react:mainfrom
cortinico:export-D45566473

Conversation

@cortinico

Copy link
Copy Markdown
Contributor

Summary:
This class is public but annotated with VisibleForTesting which is not correct (as there are no tests for it).
Apparently this class is used only within this package so it's safe to reduce its visibility to package internal.

On top of this, we had methods which were not annotated with Override, which I've fixed + I've sorted the methods to have first all the public/Override ones and the then others after.

Changelog:
[Android] [Removed] - Reduce visibility of DevInternalSettings class

Reviewed By: mdvacca

Differential Revision: D45566473

@facebook-github-botfacebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner fb-exported labels May 4, 2023
@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D45566473

Summary:
Pull Request resolved: react#37256
This class is public but annotated with VisibleForTesting which is not correct (as there are no tests for it).
Apparently this class is used only within this package so it's safe to reduce its visibility to package internal.
On top of this, we had methods which were not annotated with Override, which I've fixed + I've sorted the methods to have first all the public/Override ones and the then others after.
Changelog:
[Android] [Removed] - Reduce visibility of DevInternalSettings class
Reviewed By: mdvacca
Differential Revision: D45566473
fbshipit-source-id: 41cc0d584c328125a7866e15b483b2206958cd80
@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D45566473

@analysis-bot

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a8,639,012+8
androidhermesarmeabi-v7a7,949,799+7
androidhermesx869,126,361+14
androidhermesx86_648,979,754+3
androidjscarm64-v8a9,202,927+6
androidjscarmeabi-v7a8,391,175+11
androidjscx869,261,620+3
androidjscx86_649,518,664+3

Base commit: 4c91e8a
Branch: main

@facebook-github-botfacebook-github-bot added the Merged This PR has been merged. label May 4, 2023
@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request has been merged in 1a9e444.

DmitriyKirakosyan added a commit to microsoft/react-native-code-push that referenced this pull request Nov 9, 2023
DevInternalSettings was enclosed in ReactNative 0.73.0-rc.2, so we need to replace it with public class DeveloperSettings.
react/react-native#37256
DmitriyKirakosyan added a commit to microsoft/react-native-code-push that referenced this pull request Jan 17, 2024
….73.0 (#2617)
* Replace DevInternalSettings with DeveloperSettings
DevInternalSettings was enclosed in ReactNative 0.73.0-rc.2, so we need to replace it with public class DeveloperSettings.
react/react-native#37256
* auto install cocoapods in tests
---------
Co-authored-by: velimir-jankovic <84719115+velimir-jankovic@users.noreply.github.com>
Co-authored-by: JiglioNero <mikhail.suendukov@akvelon.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.fb-exportedMergedThis PR has been merged.p: FacebookPartner: FacebookPartner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@cortinico@facebook-github-bot@analysis-bot