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

Grouping label's expand button with grouping id #2012

Merged
merged 4 commits into from
Aug 29, 2019

Conversation

m-masataka
Copy link
Contributor

In alerts screen, Expand/Collapse button cooperated with grouping labels.
So when several groups have the same grouping labels, this button will expand all of them. Related to #1875 .
To fix this issue, I implemented expand button with grouping id. (this id is only used UI)

Signed-off-by: m-masataka <m.mizukoshi.wakuwaku@gmail.com>
@m-masataka
Copy link
Contributor Author

alertmanager

groups
(List.indexedMap Tuple.pair groups
|> List.map
(\( index, group ) ->
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to first do indexedMap to build tuples and then map over tuples. You can call List.indexedMap with \index group ->.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

@w0rm
Copy link
Member

w0rm commented Aug 27, 2019

A small improvement, everything else looks good! I think that after this change this should also perform better, because the key type of a Set is simpler.

Thank you!

@simonpasquier
Copy link
Member

Thanks a lot @m-masataka! You need to rebase your PR after #2013 is merged.

@stuartnelson3 @mxinden once this gets merged, I suggest that we consider a new release of AlertManager as #1875 is quite annoying and has been reported in different occasions. I can sign up for preparing the new release.

@stuartnelson3
Copy link
Contributor

sounds great to me

Signed-off-by: m-masataka <m.mizukoshi.wakuwaku@gmail.com>
Signed-off-by: m-masataka <m.mizukoshi.wakuwaku@gmail.com>
Signed-off-by: m-masataka <m.mizukoshi.wakuwaku@gmail.com>
@simonpasquier simonpasquier merged commit c5d41bb into prometheus:master Aug 29, 2019
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.

4 participants