search

gruntjs / grunt-contrib-watch

Run tasks whenever watched files change.
http://gruntjs.com/
MIT License
1.98k stars 356 forks source link

Bump version due to dependencies update #495

Closed lirantal closed 8 years ago

lirantal commented 8 years ago

Requires to update the module's version for projects to get the new packages

shama commented 8 years ago

@lirantal Could you explain why we should bump all the changes pending in master on a patch version? Also do you think it's a good idea to publish a version with tests failing?

lirantal commented 8 years ago

Hi @shama

Those tests were failing before the merged PR to update the dependencies, right? And secondly, you're right about the merge with master for incrementing a version, I didn't notice you have tagged versions for the module.

However, I do think that once the build is stable we increment either the patch or the minor versions for other projects who depend on this module will get the changes.

XhmikosR commented 8 years ago

This is bad practice. The tests should be fixed and also this shouldn't be a patch version.

XhmikosR commented 8 years ago

BTW @shama if there's any issue or something related to the changes we need to backport in contrib plugins (like the things we discussed the other time), please let me know.

shama commented 8 years ago

@lirantal Please take a look at http://semver.org/ and then look at the changes in master since the last release. Could you explain why you recommend publishing on a minor or patch version? Do you think those changes will break other people's builds on next install if we publish as a patch?

@XhmikosR Thanks!

ntwb commented 8 years ago

Heres the diff between v0.6.1 and master: https://github.com/gruntjs/grunt-contrib-watch/compare/v0.6.1...master, there are enough changes albeit it only minor changes in ivereload.js/taskrun.js/taskrunner.js/watch.js to warrant a minor release in my opinion.

With that said though I agree that it should not be released until all the tests pass. (Currently tests pass on NodeJS v0.10.x, and fail on v4 & v5)

ntwb commented 8 years ago

See also #491

lirantal commented 8 years ago

@shama I'm aware of semver, but I didn't review master to trace back any changes. Looking at it now I still see a few commits related to spelling, jshints, and such which I don't believe to break the API but I didn't investigate more than a quick look and you know best.

At first I was under the impression that actually the master branch is already the last release tagged 0.6.1 and that the dependencies update is the new change.

So I take it back and we can close this PR as there are indeed tests that need to be resolved along with more commits on master rather than the package.json changes.