Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add redis multi instance monitor support for redis-sentinel #356
Add redis multi instance monitor support for redis-sentinel #356
Changes from 13 commits
e85b6e8
facd408
c87be32
a4d1b5b
dc95a13
c4230cf
4744ae6
deb4075
68f44c4
1a4e9b6
7e811fc
dafbd47
c213e88
fef0597
c09d1ff
d5482b4
e6d9cef
7846856
7f53eb9
2bc9ccf
041ba16
b368f80
38d19eb
b2e1a9e
9d60184
b36cc11
444933e
e31e457
4844efb
bd0ae2e
ad146c9
0bf57f0
dc15fb9
a688036
184ccdd
88434c1
170269a
7298967
1e1e552
a8a20b7
484ce19
0fa0889
3894fa4
f417d65
204b844
e809d0a
38394bb
d2d9bcc
eed6b76
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Wouldn't it be better to define a type alias using a struct?
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.
you mean something like this:
type Redis::MasterName = Struct[{
instead of this:
type Redis::MasterName = Hash[String, Struct[{
?
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.
What do u think about this:
The puppet code will look like:
The template would be changed to:
Do you mean something like that?
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.
I didn't realize you already had a struct. I was mostly thinking about the documentation, since you specify all keys. Would it be better to document the type instead?
My suggestion would be to generate the reference (
rake strings:generate:reference
) and see how they both look like. See what makes the most sense.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.
I've changed the variable name to make clear, that we have an breaking change with this feature.
The handling is a little bit more user friendly.
And i run puppet strings to document the data type.
This file was deleted.