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

Versionist should continue even if a package.json for the updateVersion default doesn't exist #73

Open
hedss opened this issue Feb 21, 2017 · 6 comments
Assignees

Comments

@hedss
Copy link
Contributor

hedss commented Feb 21, 2017

Currently versionist will error if it uses the default npm use of updateVersion and no package.json exists in the repository it's executing in.

What should happen is a warning is returned, but the CHANGELOG is still correctly updated.

Without this functionality, every non-Node based repo currently needs a custom versionist.conf.js which is far from ideal (see https://github.com/resin-io/resin-containers for an example, which has an empty callback).

@lurch
Copy link
Contributor

lurch commented Feb 21, 2017

Is the updateVersion setting in resin-containers's versionist.conf.js the only difference from versionist's built-in config file? Would it make sense for versionist to apply values from its built-in config file for any settings not found in the current directory's versionist.conf.js? Or is that a nonsensical suggestion which would be too backwards-incompatible?

@hedss
Copy link
Contributor Author

hedss commented Feb 21, 2017

I don't think this would be a good idea, mainly as it might apply settings for things specifically not required. Our real issue here is that the default assumes NPM and errors on non-NPM. I would expect it to simply not care if there was no package.json.

@craigmulligan
Copy link
Contributor

craigmulligan commented Feb 21, 2017

Agreed, it should just log no package.json rather than throwing.

@lurch
Copy link
Contributor

lurch commented Feb 22, 2017

So just to clarify, if the (built-in) default updateVersion setting for versionist is 'npm', and there's no versionist.conf.js in the current directory, and also no package.json in the current directory, then versionist should treat updateVersion as (cwd, version, cb) => { cb(); } ?
Having just had a quick look at the versionist README, rather than overriding updateVersion to (cwd, version, cb) => { cb(); }, would it be cleaner to instead override editVersion to false?

And IMHO this behaviour should only be triggered if there's no local versionist.conf.js and also no local package.json. If there is a local versionist.conf.js (specifiying updateVersion: 'npm') but no local package.json then that should error, because there's no reason not to just update the local versionist.conf.js to suit.

@hedss
Copy link
Contributor Author

hedss commented Feb 22, 2017

That's a good point; editVersion as false would have been a lot cleaner!
I also agree that if there is an overriding versionist.conf.js which specifies project update and there's no relevant file (such as package.json) then that is definitely an error.

So, in conclusion:

  • The default internal versionist.conf.js should be set to updateVersion: npm as it is at the moment. Should no package.json be found in the repo using this default, then a verbose warning occurs, but versionist completes successfully
  • Should an overriding versionist.conf.js be found in the repo, then this is used instead. At this point, any non-conforming repo will cause versionist to throw an error should any of the requirements in the config not hold true

@lurch
Copy link
Contributor

lurch commented Jan 31, 2018

This also connects to the discussion in #84

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

No branches or pull requests

4 participants