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 fsevents from optionalDependencies #6792

Merged

Conversation

nerrad
Copy link
Contributor

@nerrad nerrad commented May 17, 2018

Description

In #6772 fsevents was added as an optional dependency to the package.json. I don't know why, but this broke our usage of gutenberg as a devDepdency for tests.

See this travis build for the example pass before the pull (6772) was merged and this travis build for an example fail since the pull (6772) was merged.

How has this been tested?

I tested this by running our build against the forked version of GB (see here) and verified via the resulting travis build job that removing the fsevents from optionalDependencies fixed things for us.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@nerrad
Copy link
Contributor Author

nerrad commented May 17, 2018

cc @ntwb

@ntwb ntwb self-assigned this May 17, 2018
@gziolo gziolo added the [Type] Build Tooling Issues or PRs related to build tooling label May 17, 2018
@ntwb
Copy link
Member

ntwb commented May 18, 2018

Tentatively approving and merging this PR for now

I'll revisit after some further feedback from npm

See also Slack chat https://wordpress.slack.com/archives/C02QB2JS7/p1526628875000089

@gziolo gziolo merged commit 1b1ca4b into WordPress:master May 18, 2018
@gziolo gziolo added this to the 2.9 milestone May 18, 2018
ntwb added a commit that referenced this pull request May 18, 2018
gziolo pushed a commit that referenced this pull request May 18, 2018
@nerrad nerrad deleted the BUG-try-removing-optional-dependencies branch May 18, 2018 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants