-
Notifications
You must be signed in to change notification settings - Fork 34
Conversation
LGTM! I have a legacy account and spent a few days reverting back to the 0.1.x branch of this module on some recently provisioned machines that picked up the 2.x versions. I'll be trying out the writeToLegacy option with 2.x soon. |
@bryanmikaelian going to revisit this to highlight that it is better to use 1.x if the account isn't on tags. |
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.
@bryanmikaelian elaborated a little more on when to use writeToLegacy
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. +1 for clarification. Do you think it is worth mentioning that this flag is there to help users migrate to tags but isn't necessarily a requirement if your account supports sources and tags i.e the user that just wants to make the jump to tags and doesn't care about their sources-based data?
@niklibrato If |
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. I'll publish this to NPM, after we merge, so the docs are update here: https://www.npmjs.com/package/statsd-librato-backend
@bryanmikaelian good point elaborating a little more. |
Just realized that the options part is completely messed up. I'll fix it. |
@bryanmikaelian @gmckeever can you please review? |
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.
💯 ✅
@stevenscg if your Librato account doesn't support tags, we recommend you use the 1.x branch rather than the writeToLegacy option. |
@niklibrato Got it. Thanks. |
No description provided.