Skip to content

Conversation

@gssbzn
Copy link
Contributor

@gssbzn gssbzn commented Apr 3, 2020

No description provided.

@gssbzn gssbzn requested a review from andreaangiolillo April 6, 2020 09:21
Copy link
Collaborator

@andreaangiolillo andreaangiolillo left a comment

Choose a reason for hiding this comment

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

LGTM

@gssbzn
Copy link
Contributor Author

gssbzn commented Apr 6, 2020

@andreaangiolillo fixed some copy/pasta

Copy link
Collaborator

@andreaangiolillo andreaangiolillo left a comment

Choose a reason for hiding this comment

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

LGTM

@gssbzn
Copy link
Contributor Author

gssbzn commented Apr 6, 2020

@themantissa ready for you and your team

themantissa
themantissa previously approved these changes Apr 7, 2020
Copy link
Collaborator

@themantissa themantissa left a comment

Choose a reason for hiding this comment

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

Just a note about a confusing comment but otherwise LGTM. Will add DoU reviewers.


var _ IndexesService = &IndexesServiceOp{}

// IndexConfiguration represents current value of the metric that triggered the alert. Only present for alerts of type HOST_METRIC.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a nit but this comment is confusing - this has nothing to do with metrics, yes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, probably copy/pasta, I'll fix this

@themantissa
Copy link
Collaborator

@marinsalinas @PacoDw @coderGo93 - can one of you give a sanity check review? Sadly it's unlikely we'll use this w/ Terraform as the post with no response won't work well with the Terraform way.

PacoDw
PacoDw previously approved these changes Apr 8, 2020
Copy link
Contributor

@PacoDw PacoDw left a comment

Choose a reason for hiding this comment

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

LGTM!

@gssbzn gssbzn dismissed stale reviews from PacoDw, themantissa, and andreaangiolillo via cf16fc0 April 8, 2020 13:57
@gssbzn gssbzn requested a review from andreaangiolillo April 8, 2020 14:22
Copy link
Collaborator

@andreaangiolillo andreaangiolillo left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@andreaangiolillo andreaangiolillo left a comment

Choose a reason for hiding this comment

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

LGTM

@gssbzn gssbzn merged commit 57b3e94 into mongodb:master Apr 8, 2020
@gssbzn gssbzn deleted the CLOUDP-60028 branch April 8, 2020 15:35
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