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

Add team to trigger monitoring record PROD-2741 #110

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

tnnrj
Copy link
Contributor

@tnnrj tnnrj commented Jun 14, 2024

Adds a team value to the monitoring table and associated views.

Tested and working locally

@tnnrj tnnrj force-pushed the PROD-2741-add-monitoring-team branch from 5363a0f to 7bcc843 Compare June 14, 2024 22:59
Copy link

@alangshall alangshall left a comment

Choose a reason for hiding this comment

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

Looks generally good to me, however, my scala is not the best, so it's probably worth soliciting another approval before merging.

Approved.

@tnnrj tnnrj force-pushed the PROD-2741-add-monitoring-team branch from 7bcc843 to 0ff39b6 Compare June 18, 2024 18:23
@sriraamas sriraamas self-assigned this Jun 18, 2024
@sriraamas sriraamas self-requested a review June 18, 2024 18:51
""")
prepared.setString(1, triggerKey.getName)
prepared.setString(2, triggerKey.getGroup)
prepared.setInt(3, triggerMonitoringPriority.id)
prepared.setInt(4, maxSecondsInError)
monitoringTeam match {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

monitoringTeam.fold(prepared.setNull(5, java.sql.Types.VARCHAR))(team => prepared.setString(5, team))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I generally tend to agree with this article by jhenline that argues that Option#fold is inconsistent and other ways of writing it are more readable

@tnnrj tnnrj merged commit 7d03de5 into master Jun 20, 2024
1 check passed
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