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

refactor(metrics) record function is not needed as both calls are always made sequentially #8650

Merged
merged 1 commit into from
Feb 17, 2023
Merged

refactor(metrics) record function is not needed as both calls are always made sequentially #8650

merged 1 commit into from
Feb 17, 2023

Conversation

cscetbon
Copy link
Contributor

@cscetbon cscetbon commented Feb 5, 2023

Problem

MonitorCacheHealth uses an internal function to record metrics. 2 calls are done always sequentially to this method. It always handles a case where the wrong type is used but that would never happen.

Solution

Get rid of that function and inline both calls sequentially

@cscetbon cscetbon marked this pull request as ready for review February 5, 2023 04:41
@mangalaman93
Copy link
Member

Thanks @cscetbon for the PR. I have enabled CI run for this PR, if all the test passes, we could merge this.

@cscetbon
Copy link
Contributor Author

cscetbon commented Feb 5, 2023

if all the test passes, we could merge this.

Unfortunately the same thing has been preventing PRs from being merged for the last 2 weeks because of an issue with sending coverage results which had nothing to do with those PRs

@mangalaman93
Copy link
Member

We are looking into and we should either find a way around or fix it head on. I will keep you posted. Thanks

@mangalaman93
Copy link
Member

We are still looking at this issue, give us another few days to figure it out.

@akon-dey akon-dey merged commit 043a1cf into hypermodeinc:main Feb 17, 2023
@mangalaman93
Copy link
Member

@cscetbon Thanks for your contribution :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants