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 #13: Upgrade glob to 5.x for nodir option #14

Merged
merged 1 commit into from
Jun 6, 2015

Conversation

yurenju
Copy link
Contributor

@yurenju yurenju commented May 11, 2015

No description provided.

@yurenju
Copy link
Contributor Author

yurenju commented May 12, 2015

at the end I change supported versions to 0.10 & 0.12. @mklabs, what do you think? In my opinion, support the latest two versions is reasonable.

@mklabs
Copy link
Owner

mklabs commented Jun 6, 2015

Hello @yurenju

Sounds good to me. Thanks and sorry for the delay.

mklabs added a commit that referenced this pull request Jun 6, 2015
Fix #13: Upgrade glob to 5.x for nodir option
@mklabs mklabs merged commit 95a748d into mklabs:master Jun 6, 2015
@dougwilson
Copy link

FYI releasing a patch version that drops support for runtime versions is pretty lame :(

@mklabs
Copy link
Owner

mklabs commented Jun 7, 2015

@dougwilson What do you mean ? Is it breaking anything for you ?

@dougwilson
Copy link

Yes, notably old versions of istanbul are no longer installable on Node.js 0.8, since they depended on 0.1.x of this lib; it also seems like this PR was adding a feature, which seems more fit for a minor version bump anyway.

@yurenju
Copy link
Contributor Author

yurenju commented Jun 7, 2015

consider the situation which @dougwilson have, a possible solution is recall 0.1.6 and bump it to 2.0.0 but not sure if it's a proper solution for a minor change.

I also checked node-glob's travis they only run test & build on node.js, 0.10, 0.12 & io.js, istanbul run test & build on node.js 0.8, 0.10 & 0.12

@mklabs
Copy link
Owner

mklabs commented Jun 8, 2015

Thanks guys for the feedback. I'll check today if we can re-add compatibility to 0.8, considering istanbul targeting also 0.8.

It seemed like a minor change so we bump a minor version @dougwilson but you're right. I'll try to fix that later today.

@mklabs mklabs mentioned this pull request Jun 8, 2015
@dougwilson
Copy link

It seemed like a minor change so we bump a minor version

Yea, the minor number is the middle number :)

@mklabs
Copy link
Owner

mklabs commented Jun 13, 2015

Hi guys,

I can finally work on this issue, sorry for the delay.

@yurenju I think I'll revert your change and bump a new 0.1.x version, then reapply your commit to bump to 0.2.x

I also want to add a synchronous api, might be handy, will probably do along the 0.2.x versions

@dougwilson Thanks for the version issue and your advices, much appreciated.

@popomore
Copy link
Contributor

I think you can bump 0.2.x for new version of istanbul now, and add new feature in the following version.

@dougwilson
Copy link

Thank you @mklabs I just tested that old instanbuls are once again installable :) I really appreciate it!

@yurenju
Copy link
Contributor Author

yurenju commented Jun 14, 2015

Great! thanks @mklabs & @dougwilson!

@mklabs
Copy link
Owner

mklabs commented Jun 14, 2015

Thanks to both of you. It was a bit tedious but we did it! 👍

by the way, if you ever need it, I introduced a sync API if you have any feedbacks, they're welcome. I still need to pass in options to glob / minimatch

@popomore
Copy link
Contributor

@mklabs Have you bumped 0.2.x to npm?

@mklabs
Copy link
Owner

mklabs commented Jun 14, 2015

I should have, I didn't ?

https://www.npmjs.com/package/fileset apparently no :) I'll do that right away

@mklabs
Copy link
Owner

mklabs commented Jun 14, 2015

published

@popomore
Copy link
Contributor

Great!

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.

4 participants