Skip to content

Conversation

@hildjj
Copy link
Collaborator

@hildjj hildjj commented Oct 17, 2016

Sorry for the large-ish PR, but this is functionality that needed to all go together.

new capabilities:

optional-dev-dependency -S camelcase  // install camelcase and store it in package.json
optional-dev-dependency --save camelcase -t foo // also save a tag
optional-dev-dependency -t foo // install only packages with the foo tag

I also fixed a major bug. The existing code tended to check for dependencies in the node_modules of this module, rather than the one in which it was executing.

This should be backward-compatible both on the command line as well as the API, so you could do a minor version bump if you liked.

@coveralls
Copy link

Coverage Status

Coverage increased (+3.6%) to 92.537% when pulling f65f53e on hildjj:package into 99136ba on bcoe:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+3.6%) to 92.537% when pulling f65f53e on hildjj:package into 99136ba on bcoe:master.

@coveralls
Copy link

coveralls commented Oct 17, 2016

Coverage Status

Coverage increased (+3.6%) to 92.537% when pulling 3c938c6 on hildjj:package into 99136ba on bcoe:master.

@hildjj
Copy link
Collaborator Author

hildjj commented Oct 17, 2016

OK, I fixed the build. Done playing with it. :)

@bcoe
Copy link
Owner

bcoe commented Oct 17, 2016

@hildjj this is looking awesome! appreciate the huge amount of work. One immediate comment, mind updating the README with some of the new features you've added, this will aid me in my code review.

@hildjj
Copy link
Collaborator Author

hildjj commented Oct 17, 2016

On it.

@coveralls
Copy link

coveralls commented Oct 18, 2016

Coverage Status

Coverage increased (+3.6%) to 92.537% when pulling 46a61e5 on hildjj:package into 99136ba on bcoe:master.

@hildjj
Copy link
Collaborator Author

hildjj commented Oct 18, 2016

Note: I'm also willing to migrate the existing tests to use the tempdir approach, so that the tests don't modify the node_modules directory of this package -- which would be cleaner.

@hildjj
Copy link
Collaborator Author

hildjj commented Oct 18, 2016

Sorry for the noise. I'm finding stuff as I start to use the new features in another project. Let me know if you want me to consolidate down to a single patch.

@coveralls
Copy link

coveralls commented Oct 18, 2016

Coverage Status

Coverage increased (+3.3%) to 92.143% when pulling dbf7490 on hildjj:package into 99136ba on bcoe:master.

@hildjj
Copy link
Collaborator Author

hildjj commented Oct 18, 2016

See https://github.com/hildjj/node-cbor/blob/optional/package.json for a working example of how I intend this to be used.

@hildjj
Copy link
Collaborator Author

hildjj commented Oct 24, 2016

Status? I'm about to put this into production for node-cbor, and I'd much rather point to a normal dependency than to my github repo.

@bcoe bcoe merged commit b9c78d7 into bcoe:master Oct 25, 2016
@bcoe
Copy link
Owner

bcoe commented Oct 25, 2016

@hildjj sorry about the slow turn around. I've published optional-dev-dependency@1.4.0, take it for a spin.

I've also added you as an owner to optional-dev-dependency on npmjs.com.

@hildjj hildjj deleted the package branch October 25, 2016 19:44
@hildjj
Copy link
Collaborator Author

hildjj commented Oct 25, 2016

Thanks!

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.

3 participants