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

Library version #1042

Closed
vitaly-t opened this issue Mar 13, 2016 · 11 comments
Closed

Library version #1042

vitaly-t opened this issue Mar 13, 2016 · 11 comments

Comments

@vitaly-t
Copy link
Contributor

@vitaly-t vitaly-t commented Mar 13, 2016

Considering the following facts about this library:

  • Huge user base
  • Used on both server-side and client-side
  • Frequent update schedule

It is asking to extend the library's root with a version object, similar to that of Angular JS:

require('bluebird').version = {
    name: 'false-positives-slayer',
    full: '3.3.4',
    major: 3,
    minor: 3,
    build: 4
};

It will help with the diagnostics in libraries built on extensive use of bluebird.

@benjamingr
Copy link
Collaborator

@benjamingr benjamingr commented Mar 13, 2016

Any example where this kind of detection is necessary?

@vitaly-t
Copy link
Contributor Author

@vitaly-t vitaly-t commented Mar 13, 2016

We are logging library versions whenever an error occurs on the server side, and sometimes they occur as a result of a package update, which one can easily see then. It's much easier to log things this way when your main library provides the version information.

And whenever we debug an Angular app on the client side, we always type in the console: angular.version, to make sure the version loaded is exactly what we are expecting. It would be nice to be able to see the same for bluebird, as we are using it in conjunction with Angular JS.

@petkaantonov
Copy link
Owner

@petkaantonov petkaantonov commented Mar 14, 2016

I presume .version = "3.3.4"; is enough as that other info can be derived from it as needed

@vitaly-t
Copy link
Contributor Author

@vitaly-t vitaly-t commented Mar 14, 2016

@petkaantonov probably, but I showed how it is used in Angular.js ;) something for the lazies ;)

@vitaly-t
Copy link
Contributor Author

@vitaly-t vitaly-t commented Mar 14, 2016

@petkaantonov on the second thought though, no. You should include the version properties, because those make it possible to easily code around new library features, without having to parse the version every time.

One would use property full for a quick look up or for logging, and use properties major, minor and build when coding around some new features and/or fixes in the library ;)

@vitaly-t
Copy link
Contributor Author

@vitaly-t vitaly-t commented Mar 14, 2016

@petkaantonov also, it would be a good idea to extend every error object thrown by the library with this property version ;)

This will pay you back, as each error would include the exact version, which people can use automatically when logging issues ;)

@vitaly-t
Copy link
Contributor Author

@vitaly-t vitaly-t commented Mar 14, 2016

Something like this:

function Version() {
    this.name = 'false-positives-slayer';
    this.major = 3;
    this.minor = 3;
    this.build = 4;
    this.full = this.major + '.' + this.minor + '.' + this.build;

    Object.lock(this); // lock the instance fully.
}

// returns the full version as the default console output;
Version.prototype.inspect = function () {
    return this.full;
};

// compare to a version string to simplify
// conditional coding, and return:
//
// 0, if equal, -1 if less, 1 if greater.
Version.prototype.compare = function (ver) {
};

var ver = new Version(); // global version instance;
@petkaantonov
Copy link
Owner

@petkaantonov petkaantonov commented Mar 15, 2016

Yeah I think it's unreasonable to expect all that to be implemented in bluebird. The users who really need more than a string can parse the string easily into such object using the semver module.

@overlookmotel
Copy link
Contributor

@overlookmotel overlookmotel commented Apr 25, 2016

For my purposes, just the full version string would be easily sufficient. But it would be really useful to have a .version property available. Some libraries allow a Promise implementation to be injected (i.e. "please return promises of this type") and it'd be useful to be able to know inside the library the version of bluebird you're dealing with e.g. to work around the different syntax of Promise.promisify() in v2.x and v3.x.

I'm not sure if the .version property is best added by something like this in the main codebase: Promise.version = require('../package.json').version or by adding it in the build step.

Please let me know your preference, and I'll submit a PR.

@vitaly-t
Copy link
Contributor Author

@vitaly-t vitaly-t commented Apr 25, 2016

@overlookmotel the property should definitely be available from the library's root:

var ver = require('bluebird').version;

This is how it works in other libraries, like Angular.js: angular.version.

@overlookmotel
Copy link
Contributor

@overlookmotel overlookmotel commented Apr 25, 2016

@vitaly-t Yeah, I agree. But my question was what's the best way to implement it internally. i.e. how to set the value within bluebird, not access the value from bluebird.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.