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

Make AMKey serializable via string type #27

Merged
merged 2 commits into from
May 21, 2019
Merged

Conversation

replay
Copy link
Contributor

@replay replay commented May 17, 2019

Other minor changes:

  • Fixes typo in error message
  • Updates stringer generated function for agg method

Related: grafana/metrictank#1291

@replay replay requested a review from Dieterbe May 17, 2019 15:12
@replay replay force-pushed the serializable_amkey branch from bc3bfad to 2004982 Compare May 17, 2019 15:24
other minor changes:
- fixes typo in error message
- updates stringer generated function for agg method
Copy link

@DanCech DanCech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable, would it make sense to pull the special value 97 out into a dedicated const AMKeyExtensionId int8 = 97 to avoid duplicating it?

@replay
Copy link
Contributor Author

replay commented May 20, 2019

yes, that would make sense. doing that

update: done
@DanCech please take another look

@replay replay merged commit 49c3f30 into master May 21, 2019
@replay replay deleted the serializable_amkey branch May 21, 2019 12:31
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

Successfully merging this pull request may close these issues.

3 participants