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.

Sign up for GitHub

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

Jump to bottom

fix: try to fix the max satisfy version (n.x) in grandfather's deps #50

Merged
merged 1 commit into from
Mar 11, 2016
Merged

fix: try to fix the max satisfy version (n.x) in grandfather's deps #50

merged 1 commit into from
Mar 11, 2016

Conversation

fengmk2
Copy link
Member

@fengmk2 fengmk2 commented Mar 11, 2016

closes #49

@fengmk2 fengmk2 added this to the 1.x milestone Mar 11, 2016
@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @dead-horse to be a potential reviewer

@fengmk2
Copy link
Member Author

fengmk2 commented Mar 11, 2016

image

@codecov-io
Copy link

Current coverage is 93.34%

Merging #50 into master will decrease coverage by -0.03% as of b883a73

@@            master     #50   diff @@
======================================
  Files           18      18       
  Stmts          755     766    +11
  Branches       132     138     +6
  Methods          0       0       
======================================
+ Hit            705     715    +10
  Partial          0       0       
- Missed          50      51     +1

Review entire Coverage Diff as of b883a73

Powered by Codecov. Updated on successful CI builds.

fengmk2 added a commit that referenced this pull request Mar 11, 2016
@fengmk2
Merge pull request #50 from cnpm/try-to-use-exists-version
302ab3a 
fix: try to fix the max satisfy version (n.x) in grandfather's deps
@fengmk2 fengmk2 merged commit 302ab3a into master Mar 11, 2016
@fengmk2 fengmk2 deleted the try-to-use-exists-version branch March 11, 2016 16:41
@fengmk2
Copy link
Member Author

fengmk2 commented Mar 11, 2016

1.3.2

@fengmk2
Copy link
Member Author

fengmk2 commented Mar 11, 2016

试了很多项目，就只有 antd 会这么使用

dead-horse
dead-horse reviewed Mar 12, 2016
View reviewed changes
lib/install.js
@@ -142,6 +143,26 @@ function* _install(parentDir, pkg, options) {
if (bundledDependencies.indexOf(childPkg.name) !== -1) {
continue;
}
// if version format "n.x", check grandfather's dependencies
Copy link
Member

Choose a reason for hiding this comment

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

这个逻辑怎么看怎么恶心。。

Copy link
Contributor

Choose a reason for hiding this comment

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

和 n.x 没关系吧，其他写法也有一样的问题。本质还是顺序问题。优先安装父级的相同依赖模块版本。

Copy link
Member

Choose a reason for hiding this comment

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

npm/npm#11887

@popomore popomore mentioned this pull request Dec 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@dead-horse dead-horse

Labels
bug enhancement
Projects
None yet
Milestone
1.x
Development

Successfully merging this pull request may close these issues.

Find the max satisfy version (n.x) in grandfather's deps
6 participants
@fengmk2 @mention-bot @codecov-io @yiminghe @afc163 @dead-horse