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

Remove jest and jest-junit from runtime dependencies #23276

Closed

Conversation

vovkasm
Copy link
Contributor

@vovkasm vovkasm commented Feb 3, 2019

Summary

Commit 06c13b3 pretend to update jest
to new version, but also adds jest and jest-junit to runtime dependencies
list (in addition to dev dependencies).

This commit fixes this, by removing jest and jest-junit from runtime
dependeincies.

As for motivation... story is short:

  1. Updating RN 0.58.1 -> 0.58.3 I found that whole babel@6 infrastructure was returned to my node_modules (it is dangerous, because many tools like metro can found it and use it instead of babel@7).
  2. It was because of this commit: 9d19ab0 that have 130 random files changed (and provides no meaning description).
    But it points to: b864e7e (which also provides no meaning description, why it was reverted?)
    And finally points to: 696bd89, which was a "sync with master" commit. Ok. So it was from master.
  3. Then I checked that jest still in runtime dependencies on master and fixed that. Also I hope this will be cherry picked into 0.58-stable.

Changelog

[General] [Fixed] - Fix jest and jest-junit to be only development dependencies

Test Plan

I'm already have this change in my fork of 0.58-stable branch, all worked as expected.

Commit 06c13b3 pretend to update jest
to new version, but also adds jest and jest-unit to runtime dependencies
list (in addition to dev dependencies).

This commit fixes this, by removing jest and jest-junit from runtime
dependeincies.
@vovkasm vovkasm requested a review from hramos as a code owner February 3, 2019 08:38
@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 Feb 3, 2019
@pull-bot
Copy link

pull-bot commented Feb 3, 2019

Warnings
⚠️

🔒 package.json - Changes were made to package.json. This will require a manual import by a Facebook employee.

Generated by 🚫 dangerJS

@vovkasm
Copy link
Contributor Author

vovkasm commented Feb 3, 2019

@rafeca As one who initially changed runtime dependency, can you please help with this to be landed?

Copy link
Contributor

@cpojer cpojer left a comment

Choose a reason for hiding this comment

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

Oops! Thanks for the fix.

@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 Feb 4, 2019
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.

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

@grabbou
Copy link
Contributor

grabbou commented Feb 4, 2019

@vovkasm

It was because of this commit: 9d19ab0 that have 130 random files changed (and provides no meaning description).

This commit brought the following commits to 0.58-stable branch. Github UI was showing they were already part of the branch, where in fact, they were not. This was done to avoid confusion and bring it to sync.

de60e86...8f283b9

@react-native-bot
Copy link
Collaborator

@vovkasm merged commit c7b57f1 into facebook:master.

@facebook facebook locked as resolved and limited conversation to collaborators Feb 4, 2019
@react-native-bot react-native-bot added the Merged This PR has been merged. label Feb 4, 2019
grabbou pushed a commit that referenced this pull request Feb 4, 2019
Summary:
Commit 06c13b3 pretend to update jest
to new version, but also adds jest and jest-junit to runtime dependencies
list (in addition to dev dependencies).

This commit fixes this, by removing jest and jest-junit from runtime
dependeincies.

As for motivation... story is short:
1. Updating RN 0.58.1 -> 0.58.3 I found that whole babel@6 infrastructure was returned to my `node_modules` (it is dangerous, because many tools like metro can found it and use it instead of babel@7).
2. It was because of this commit: 9d19ab0 that have 130 random files changed (and provides no meaning description).
But it points to: b864e7e (which also provides no meaning description, why it was reverted?)
And finally points to: 696bd89, which was a "sync with master" commit. Ok. So it was from master.
3. Then I checked that jest still in runtime dependencies on master and fixed that. Also I hope this will be cherry picked into 0.58-stable.

[General] [Fixed] - Fix jest and jest-junit to be only development dependencies
Pull Request resolved: #23276

Differential Revision: D13941275

Pulled By: cpojer

fbshipit-source-id: a6f3377e670554b21f3ebd2f12d33e29d2969df1
@hramos hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 6, 2019
matt-oakes pushed a commit to matt-oakes/react-native that referenced this pull request Feb 7, 2019
Summary:
Commit 06c13b3 pretend to update jest
to new version, but also adds jest and jest-junit to runtime dependencies
list (in addition to dev dependencies).

This commit fixes this, by removing jest and jest-junit from runtime
dependeincies.

As for motivation... story is short:
1. Updating RN 0.58.1 -> 0.58.3 I found that whole babel@6 infrastructure was returned to my `node_modules` (it is dangerous, because many tools like metro can found it and use it instead of babel@7).
2. It was because of this commit: 9d19ab0 that have 130 random files changed (and provides no meaning description).
But it points to: b864e7e (which also provides no meaning description, why it was reverted?)
And finally points to: 696bd89, which was a "sync with master" commit. Ok. So it was from master.
3. Then I checked that jest still in runtime dependencies on master and fixed that. Also I hope this will be cherry picked into 0.58-stable.

[General] [Fixed] - Fix jest and jest-junit to be only development dependencies
Pull Request resolved: facebook#23276

Differential Revision: D13941275

Pulled By: cpojer

fbshipit-source-id: a6f3377e670554b21f3ebd2f12d33e29d2969df1
t-nanava pushed a commit to microsoft/react-native-macos that referenced this pull request Jun 17, 2019
Summary:
Commit 06c13b3 pretend to update jest
to new version, but also adds jest and jest-junit to runtime dependencies
list (in addition to dev dependencies).

This commit fixes this, by removing jest and jest-junit from runtime
dependeincies.

As for motivation... story is short:
1. Updating RN 0.58.1 -> 0.58.3 I found that whole babel@6 infrastructure was returned to my `node_modules` (it is dangerous, because many tools like metro can found it and use it instead of babel@7).
2. It was because of this commit: 9d19ab0 that have 130 random files changed (and provides no meaning description).
But it points to: b864e7e (which also provides no meaning description, why it was reverted?)
And finally points to: 696bd89, which was a "sync with master" commit. Ok. So it was from master.
3. Then I checked that jest still in runtime dependencies on master and fixed that. Also I hope this will be cherry picked into 0.58-stable.

[General] [Fixed] - Fix jest and jest-junit to be only development dependencies
Pull Request resolved: facebook#23276

Differential Revision: D13941275

Pulled By: cpojer

fbshipit-source-id: a6f3377e670554b21f3ebd2f12d33e29d2969df1
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.

7 participants