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

Fix packager asset requests on windows #11254

Closed

Conversation

janicduplessis
Copy link
Contributor

6554ad5 broke assets on windows, this fixes it and add a test to avoid regressions.

Ideally we'd run all the Dependency graph tests on both posix and win32 filesystems but that would probably require doing some sort of factory function to create the tests because I don't think we want to duplicate every test (file is big enough already :)). So for now I just copied that one test and changed the paths manually.

Test plan
Run the new test without the fix -> fails
Run the new test with the fix -> succeeds

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @cpojer and @davidaurelio to be potential reviewers.

@cpojer
Copy link
Contributor

cpojer commented Dec 1, 2016

Thanks @janicduplessis for being on top of things.

@facebook-github-bot facebook-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. GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Dec 1, 2016
@facebook-github-bot
Copy link
Contributor

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

robclouth pushed a commit to robclouth/react-native that referenced this pull request Dec 7, 2016
Summary:
6554ad5 broke assets on windows, this fixes it and add a test to avoid regressions.

Ideally we'd run all the Dependency graph tests on both posix and win32 filesystems but that would probably require doing some sort of factory function to create the tests because I don't think we want to duplicate every test (file is big enough already :)). So for now I just copied that one test and changed the paths manually.

**Test plan**
Run the new test without the fix -> fails
Run the new test with the fix -> succeeds
Closes facebook#11254

Differential Revision: D4265157

Pulled By: cpojer

fbshipit-source-id: 511470276bd950c2943e94c2dce6840df0fe6d69
mkonicek pushed a commit that referenced this pull request Dec 12, 2016
Summary:
6554ad5 broke assets on windows, this fixes it and add a test to avoid regressions.

Ideally we'd run all the Dependency graph tests on both posix and win32 filesystems but that would probably require doing some sort of factory function to create the tests because I don't think we want to duplicate every test (file is big enough already :)). So for now I just copied that one test and changed the paths manually.

**Test plan**
Run the new test without the fix -> fails
Run the new test with the fix -> succeeds
Closes #11254

Differential Revision: D4265157

Pulled By: cpojer

fbshipit-source-id: 511470276bd950c2943e94c2dce6840df0fe6d69
DanielMSchmidt pushed a commit to DanielMSchmidt/react-native that referenced this pull request Jan 4, 2017
Summary:
6554ad5 broke assets on windows, this fixes it and add a test to avoid regressions.

Ideally we'd run all the Dependency graph tests on both posix and win32 filesystems but that would probably require doing some sort of factory function to create the tests because I don't think we want to duplicate every test (file is big enough already :)). So for now I just copied that one test and changed the paths manually.

**Test plan**
Run the new test without the fix -> fails
Run the new test with the fix -> succeeds
Closes facebook#11254

Differential Revision: D4265157

Pulled By: cpojer

fbshipit-source-id: 511470276bd950c2943e94c2dce6840df0fe6d69
cpojer pushed a commit to facebook/metro that referenced this pull request Jan 26, 2017
Summary:
6554ad5983ae85212684a09869e2ea2e0b743dd3 broke assets on windows, this fixes it and add a test to avoid regressions.

Ideally we'd run all the Dependency graph tests on both posix and win32 filesystems but that would probably require doing some sort of factory function to create the tests because I don't think we want to duplicate every test (file is big enough already :)). So for now I just copied that one test and changed the paths manually.

**Test plan**
Run the new test without the fix -> fails
Run the new test with the fix -> succeeds
Closes facebook/react-native#11254

Differential Revision: D4265157

Pulled By: cpojer

fbshipit-source-id: 511470276bd950c2943e94c2dce6840df0fe6d69
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. Import Started This pull request has been imported. This does not imply the PR has been approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants