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

Move and Rename custom propType definitions in LayoutPropTypes #21370

Closed
wants to merge 6 commits into from

Conversation

nissy-dev
Copy link
Contributor

related #21342

TODO

  • move LayoutPropType.js
  • fix flow error

CheckList

  • yarn prettier
  • yarn flow-check-android
  • yarn flow-check-ios

Test Plan

All flow checks pass.

Release Notes

[GENERAL] [ENHANCEMENT] [DeprecatedLayoutPropTypes.js] - Created.
[GENERAL] [ENHANCEMENT] [StyleSheetTypes.js] - add comments.

@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 Sep 27, 2018
@elicwhite
Copy link
Member

This is so good. Thanks. I'm excited to have the comments attached to the Flow types. Can you remove the comments from the DeprecatedLayoutPropTypes file so that the comments only have one source of truth?

@pull-bot
Copy link

pull-bot commented Sep 28, 2018

Warnings
⚠️

❗ Big PR - This PR is extremely unlikely to get reviewed because it touches 1131 lines.

Generated by 🚫 dangerJS

Copy link
Member

@elicwhite elicwhite left a comment

Choose a reason for hiding this comment

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

Thanks. Looks good! Left a small comment and it looks like you have a merge conflict in ViewStylePropTypes.


const ReactPropTypes = require('prop-types');

/**
Copy link
Member

Choose a reason for hiding this comment

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

Can you move this comment to the file with Flow types as well? You can put it on top of ____LayoutStyle_Internal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed 👍

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Sep 28, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

TheSavior is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

@nd-02110114 merged commit b6b0fc1 into facebook:master.

@facebook facebook locked as resolved and limited conversation to collaborators Oct 1, 2018
@react-native-bot react-native-bot added the Merged This PR has been merged. label Oct 1, 2018
@hramos hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 6, 2019
t-nanava pushed a commit to microsoft/react-native-macos that referenced this pull request Jun 17, 2019
…ook#21370)

Summary:
related facebook#21342

TODO
* move LayoutPropType.js
* fix flow error

CheckList
 - [x] `yarn prettier`
 - [x] `yarn flow-check-android`
 - [x] `yarn flow-check-ios`

All flow checks pass.

[GENERAL] [ENHANCEMENT] [DeprecatedLayoutPropTypes.js] - Created.
[GENERAL] [ENHANCEMENT] [StyleSheetTypes.js] - add comments.
Pull Request resolved: facebook#21370

Differential Revision: D10099715

Pulled By: RSNara

fbshipit-source-id: d0515fe0d56d9ed2fde50cc0bfb75b63aded1f5d
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.

6 participants