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(package.json): don't define minichain dependency in both deps and… #3

Merged
merged 1 commit into from
May 20, 2015

Conversation

IgorMinar
Copy link
Contributor

… devDeps

Fixes #2

@pkozlowski-opensource
Copy link

Oh, I was about to open a PR for this but it looks like Igor was faster :-)
@IgorMinar I wonder why we need to do this fix for minichain but not for ministyle? This is the same situation / pb, no?

@IgorMinar
Copy link
Contributor Author

@pkozlowski-opensource good point. in tsd's case the ministyle dep is not a problem because it is required by tsd itself, so the peerdependency is actually there.

@IgorMinar
Copy link
Contributor Author

so, for the purposes of solving our particular problem, I don't think we care about ministyle.

@alexeagle
Copy link

@IgorMinar that means our backup plan is just to add an explicit dep from tsd to minichain, we don't need to fork minitable too.

Bartvds added a commit that referenced this pull request May 20, 2015
fix(package.json): don't define minichain dependency in both deps and…
@Bartvds Bartvds merged commit d513a1c into Bartvds:master May 20, 2015
@Bartvds
Copy link
Owner

Bartvds commented May 20, 2015

Sorry for the wait guys, life went another way. Lemme get back on the horse and I'll get the things sorted out.

@alexeagle
Copy link

Thank you!

On Wed, May 20, 2015 at 10:53 AM Bart van der Schoor <
notifications@github.com> wrote:

Sorry for the wait guys, life went another way. Lemme get back on the
horse and I'll get the things sorted out.


Reply to this email directly or view it on GitHub
#3 (comment).

@Bartvds
Copy link
Owner

Bartvds commented May 20, 2015

This is on npm as 0.0.4.

Now we hustle a new TSD release as apparently I pinned only this module to an exact version.. 😕

@Bartvds
Copy link
Owner

Bartvds commented May 20, 2015

I just realized I only merged the PR but not appreciated the discussion. So let me know if you also need the other peerDependency removed.

@Bartvds Bartvds mentioned this pull request May 20, 2015
7 tasks
@IgorMinar
Copy link
Contributor Author

it's weird that ministyle is both dependency and peerDependency. It's not clear what that means, but npm 2.x is currently happy. npm 3.x might break this again though, so it might be good to revisit what you actually need here. do you need ministyle to be provided by whoever is including minitable or do you just want npm to provide it to you as your own dependency?

@IgorMinar
Copy link
Contributor Author

thanks for merging the PR though! let's get a TSD release out so that this major PITA is over.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

do not have same dependency in dependencies and peerDependencies
4 participants