Log less

[Sneagan]

Summary

Log less

Type of Change

• Product feature
• Bug fix
• Performance improvement
• Refactor
• Other

Tested Environments

• Development
• Staging
• Production

Before Requesting Review

• Does your code build cleanly without any errors or warnings?
• Have you used auto closing keywords?
• Have you added tests for new functionality?
• Have validated query efficiency for new database queries?
• Have documented new functionality in README or in comments?
• Have you squashed all intermediate commits?
• Is there a clear title that explains what the PR does?
• Have you used intuitive function, variable and other naming?
• Have you requested security and/or privacy review if needed
• Have you performed a self review of this PR?

Manual Test Plan

[kdenhartog]

LGTM

[clD11]

suggestion: instead of compiling the regex on every call of the method and for each value can we just compile them once and reuse

[Sneagan]

I think the readability cost of moving the regex out of here is worse than the performance cost of recompiling it. This call flow spends most of its time waiting for i/o and regex compilation time will be negligible.

[clD11]

suggestion: could we extract this block into a function so we can test it

func redactData(body string) string {
	d := description.ReplaceAllString(body, "")
	c := country.ReplaceAllString(d, "")
	
	return strings.ReplaceAll(c, ",}", "}")
}

[Sneagan]

Good idea. Done.

[clD11]

suggestion: could we use strings.ReplaceAll(c, ",}", "}") here

[clD11]

question: do we have an example of the response we are trying to redact this from?

we could add an example response to a test, I wonder if we can use an or instead of having two separate passes or the regex

[Sneagan]

I added a test with an example object.

[clD11]

LGTM with comment.

[clD11]

suggestion: return strings.ReplaceAll(sbody, ",}", "}")

[clD11]

suggestion: I would not advocate using globals but we are already do it in that package for regex here and vars here. Calling must which can potentially panic on every request plus the overhead seems unnecessary. In the loop we are also copying every time rather than indexing the value.