Skip to content

Sort channels in monitor emails#435

Merged
kyla-m merged 3 commits into
stagingfrom
more-monitor-updates
Feb 9, 2023
Merged

Sort channels in monitor emails#435
kyla-m merged 3 commits into
stagingfrom
more-monitor-updates

Conversation

@ulbergc
Copy link
Copy Markdown
Collaborator

@ulbergc ulbergc commented Feb 9, 2023

addresses #426

@ulbergc ulbergc requested a review from kyla-m February 9, 2023 18:48
Copy link
Copy Markdown
Contributor

@kyla-m kyla-m left a comment

Choose a reason for hiding this comment

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

What is the purpose of sorting them a second time?

@ulbergc
Copy link
Copy Markdown
Collaborator Author

ulbergc commented Feb 9, 2023

The second sort was just to be safe, I guess. Theoretically an older alert with unsorted channels could have the send_alert called, although in practice that would never happen. I could remove the second sort (in get_printable_channels)

@kyla-m
Copy link
Copy Markdown
Contributor

kyla-m commented Feb 9, 2023

Yeah, it seems redundant.

@ulbergc
Copy link
Copy Markdown
Collaborator Author

ulbergc commented Feb 9, 2023

Ok, removed

Copy link
Copy Markdown
Contributor

@kyla-m kyla-m left a comment

Choose a reason for hiding this comment

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

Looks good

@kyla-m kyla-m merged commit 67b6d34 into staging Feb 9, 2023
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.

2 participants