-
-
Notifications
You must be signed in to change notification settings - Fork 232
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
Expose current Aedes version, used in aedes-stats #294
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add it to the prototype as well? aedes-stats should receive an instance of Aedes.
Also, can you add a unit test? |
I think the unit test should be in aedes-stats project. |
This should just check that the version exposed is the same one of the
package.json.
Il giorno dom 11 ago 2019 alle 10:05 Gnought <[email protected]> ha
scritto:
… I think the unit test should be in aedes-stats project.
I will add the code
doPub('version', aedesInstance.constructor.version)
in aedes-stats iterate() function
https://github.com/mcollina/aedes-stats/blob/master/stats.js#L52
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#294>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAMXY7FGLFG5ADW6HF67SDQD7B6DANCNFSM4IK24XLQ>
.
|
Add to prototype could be, but it will require an extra memory for storing version in each instance, and it is not useful for user except aedes-stats. |
My idea is also be memory-wise |
The following will not cause any addition memory allocation: Aedes.prototype.version = require('./package').version The reason for that is that it will stick the value on the prototype itself. Instances will look the value there, much like any additional function. |
test/meta.js
Outdated
|
||
var broker = aedes() | ||
if (broker.version === undefined) { | ||
t.fail('version undefined') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you check if the content is the same of package.json?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.