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

Add flowconfig to SampleApp #815

Merged
merged 1 commit into from
Apr 14, 2015
Merged

Conversation

frantic
Copy link
Contributor

@frantic frantic commented Apr 11, 2015

Added .flowconfig template (and a test for it!)

Test Plan:

$ ./scripts/e2e-test.sh

@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 Apr 11, 2015
@frantic frantic changed the title Add test target and flowconfig to SampleApp Add flowconfig to SampleApp Apr 14, 2015
@frantic
Copy link
Contributor Author

frantic commented Apr 14, 2015

@bhosmer - does .flowconfig look good?

@bhosmer
Copy link
Contributor

bhosmer commented Apr 14, 2015

@frantic LGTM - passes w/0 errors, right?

@ide
Copy link
Contributor

ide commented Apr 14, 2015

On Flow's side would it be possible to transitively include .flowconfigs of [include]'d directories and require()'d npm packages? It would let projects customize their .flowconfig files while keeping an up-to-date reference to React Native's .flowconfig and other projects' .flowconfig files if Flow gains more adoption. I opened an issue over at facebook/flow#308 if that's a better place to make progress.

frantic added a commit that referenced this pull request Apr 14, 2015
@frantic frantic merged commit 91db62b into facebook:master Apr 14, 2015
@bhosmer
Copy link
Contributor

bhosmer commented Apr 14, 2015

@ide James, note that such an include wouldn't come in handy here - the project has its own copy of everything, it's not reaching back to the original React Native dir. That said, we've talked about some sort of include feature, but it would be more a way to include raw content than a deep "subproject" sort of feature, at least for now. Will update 308 with any progress.

@ide
Copy link
Contributor

ide commented Apr 14, 2015

the project has its own copy of everything

Yeah. It would be nice if individual projects could each import one React Native .flowconfig into each of their own .flowconfig files.

facebook-github-bot pushed a commit that referenced this pull request Oct 12, 2018
Summary:
Fixes #815

Imagine a node with this style: `{ paddingHorizontal: 10, paddingStart: 0 }`.

After running layout on this node, we expect its computed `paddingStart` to be `0`. However, it is actually `10`.

Consider the expression `paddingEdgeStart.getValue() > 0.0f` in [`getLeadingPadding`](https://github.com/facebook/yoga/blob/328ec7dc4d104b42b836d5ccebff04033d045133/yoga/YGNode.cpp#L461). Why is `0` handled like a negative number rather than a positive number? I suspect this should be `>=` so `0` is handled like the positive numbers (this is how `getTrailingPadding` works).

It looks like facebook/yoga@3a82d2b?diff=unified&w=1#diff-07b4949bf42749fde386e769ff08a124 changed the operator from `>=` to `>` in `getLeadingPadding`. I suspect it was a mistake. `getTrailingPadding` still uses `>=`.

I manually verified this using the code in #815 and added some unit tests to catch this bug and other similar issues.

Adam Comella
Microsoft Corp.
Pull Request resolved: facebook/yoga#816

Reviewed By: priteshrnandgaonkar

Differential Revision: D10282617

Pulled By: shergin

fbshipit-source-id: 2ab2874ae39d9454308a020a960ace85573fe777
t-nanava referenced this pull request in microsoft/react-native-macos Jun 17, 2019
Summary:
Fixes #815

Imagine a node with this style: `{ paddingHorizontal: 10, paddingStart: 0 }`.

After running layout on this node, we expect its computed `paddingStart` to be `0`. However, it is actually `10`.

Consider the expression `paddingEdgeStart.getValue() > 0.0f` in [`getLeadingPadding`](https://github.com/facebook/yoga/blob/328ec7dc4d104b42b836d5ccebff04033d045133/yoga/YGNode.cpp#L461). Why is `0` handled like a negative number rather than a positive number? I suspect this should be `>=` so `0` is handled like the positive numbers (this is how `getTrailingPadding` works).

It looks like facebook/yoga@3a82d2b?diff=unified&w=1#diff-07b4949bf42749fde386e769ff08a124 changed the operator from `>=` to `>` in `getLeadingPadding`. I suspect it was a mistake. `getTrailingPadding` still uses `>=`.

I manually verified this using the code in #815 and added some unit tests to catch this bug and other similar issues.

Adam Comella
Microsoft Corp.
Pull Request resolved: facebook/yoga#816

Reviewed By: priteshrnandgaonkar

Differential Revision: D10282617

Pulled By: shergin

fbshipit-source-id: 2ab2874ae39d9454308a020a960ace85573fe777
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants