Skip to content
This repository was archived by the owner on Apr 1, 2020. It is now read-only.

Conversation

TalAmuyal
Copy link
Collaborator

Some small changes to oni-plugin-git-'s package.json:

  • Moved oni-api & oni-types from dependencies to peerDependencies, so it now only references to the root's node_modules rather than having its own copy.
  • Added missing rimraf to devDependencies as it is needed (I think) for the build command.

@TalAmuyal TalAmuyal self-assigned this Aug 3, 2018
@TalAmuyal TalAmuyal requested a review from akinsho August 3, 2018 05:35
@TalAmuyal TalAmuyal force-pushed the fix_local_build_issue branch from 7ad3ad2 to 7efea2d Compare August 4, 2018 07:54
akinsho
akinsho previously approved these changes Aug 4, 2018
@codecov
Copy link

codecov bot commented Aug 8, 2018

Codecov Report

Merging #2488 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2488   +/-   ##
======================================
  Coverage    44.2%   44.2%           
======================================
  Files         344     344           
  Lines       13886   13886           
  Branches     1829    1829           
======================================
  Hits         6138    6138           
  Misses       7508    7508           
  Partials      240     240

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 60c647e...c9ce792. Read the comment docs.

@TalAmuyal
Copy link
Collaborator Author

As mentioned in #2493, I tested the strange error and it occurs when using peerDependencies.
I reverted that part since it is a nice-to-have and will likely face it later when I try pluginizing more parts.

@akin909, will you re-review?

Copy link
Member

@akinsho akinsho left a comment

Choose a reason for hiding this comment

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

@TalAmuyal looks good thanks for tweaking that 👍, just checking this all still works? (I'm away atm so can't run it locally) changes look minor anyway though so should be fine

@TalAmuyal TalAmuyal merged commit a54bb2c into master Aug 9, 2018
@TalAmuyal
Copy link
Collaborator Author

Yes, it still works.
The important thing is the rimraf dependency.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants