Skip to content

Commit

Permalink
updating qs dependency
Browse files Browse the repository at this point in the history
  • Loading branch information
John Brett committed Oct 20, 2015
1 parent e72c497 commit 57c547b
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "subtext",
"description": "HTTP payload parsing",
"version": "2.0.1",
"version": "2.0.2",
"repository": "git://github.com/hapijs/subtext",
"main": "lib/index.js",
"keywords": [
Expand All @@ -18,8 +18,8 @@
"boom": "2.x.x",
"content": "1.x.x",
"hoek": "2.x.x",
"qs": "4.x.x",
"pez": "1.x.x",
"qs": "5.x.x",
"wreck": "6.x.x"
},
"devDependencies": {
Expand Down

5 comments on commit 57c547b

@jcollum
Copy link

Choose a reason for hiding this comment

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

I may be being overly critical here but bumping a major version of a dependency seems like it should be reserved for at least a minor version, not a patch version. I'm getting an NPM warning on Hapi because of this.

@johnbrett
Copy link
Contributor

Choose a reason for hiding this comment

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

The error you are seeing is a due to the version of hapi currently on npm (11.0.2) uses the version of subtext before this change was made. hapi 11.0.3 has the latest version of subtext (2.0.2), when this is published the peer dependency error will be gone, the error not related to whether a patch or minor version was used for this update.

Why do you think a minor version should be used here? A minor version is reserved for adding backwards compatible functionality.

@jcollum
Copy link

Choose a reason for hiding this comment

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

Well the next step down would be a patch version. Changing a major version of a dependency definitely doesn't seem appropriate there.

@jcollum
Copy link

Choose a reason for hiding this comment

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

I asked around at the office and everyone agrees with you. So it's just me misunderstanding it.

@johnbrett
Copy link
Contributor

Choose a reason for hiding this comment

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

No problem :) Any other questions let me know!

Please sign in to comment.