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

Adding documentation for object_per_topic_config and metric_per_topic_config #114

Merged
merged 3 commits into from
Jan 2, 2023

Conversation

wmoss
Copy link

@wmoss wmoss commented Jan 1, 2023

This adds object_per_topic_config and metric_per_topic_config to the documentation as well as an example taken from #96. It also adds a check when parsing the config to make sure both are not specified.

@wmoss
Copy link
Author

wmoss commented Jan 1, 2023

I think this should also address #86 and #99

@hikhvar hikhvar merged commit 682d2c1 into hikhvar:master Jan 2, 2023
@hikhvar
Copy link
Owner

hikhvar commented Jan 2, 2023

Thank you for your contribution!

if cfg.MQTT.ObjectPerTopicConfig != nil && cfg.MQTT.MetricPerTopicConfig != nil {
return Config{}, fmt.Errorf("only one of object_per_topic_config and metric_per_topic_config can be specified")
}

Copy link
Author

Choose a reason for hiding this comment

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

I should have said this earlier, but it is worth noting that this is potentially a breaking change for some users. They would have had an incorrect config, but if they were using it to parse json then it would have worked with both configs in there and now it will throw an error. I don't think this is a big deal, but maybe worth noting in the release notes or something.

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.

None yet

2 participants