Fix typo for CorrelatedInvocationsID #588
Conversation
This cl make correction to the identifier's CorrelatedInvocationsID field name to make it match the proto. It was CorrelatedInvocationID before. We need to have CorrelatedInvocationsID in the ToProto method to deduplicate builds that have the same invocation id.
This cl make correction to the identifier's CorrelatedInvocationsID field name to make it match the proto. It was CorrelatedInvocationID before. We need to have CorrelatedInvocationsID in the ToProto method to deduplicate builds that have the same invocation id.
|
I have tested with a re-client chrome build, this sdk change works as expected, and can pass the RBE_correlated_invocations_id value to the rpl log. |
go/pkg/contextmd/contextmd.go
Outdated
|
|
||
| // truncateStrings remove one char from the longest str at a time until the | ||
| // total length of all the strings is less than limit value. | ||
| func truncateStrings(limit int, inputs ...*string) { |
There was a problem hiding this comment.
Curious why you're passing strings by reference here? Strings in Go are backed by slices, which are cheap to copy because the underlying array is shared.
There was a problem hiding this comment.
I want to pass m's fields here, and modify them in place (basically modify m in place).
say, if I pass strings here, and the interface is func truncateStrings(limit int, inputs ...string) outputs []string{}
I will end up with very long code on the caller's side. I was keep thinking it over and over for about an hour this morning, didn't find out a better solution than pass in ...*string.
outputs := truncateStrings(32, m.a, m.b, m.c, m.d, m.e)
m.a = outputs[0]
m.b = outputs[1]
m.c = outputs[2]
m.d = outputs[3]
m.e = outputs[4]
return m
go/pkg/contextmd/contextmd.go
Outdated
| if total <= limit { | ||
| return | ||
| } | ||
| for total > limit { |
There was a problem hiding this comment.
This loop might end up being expensive considering it's executed at least once for every action. I think it's safer to limit this PR to what's needed to satisfy its title and leave this refactor to another one.
This cl make correction to the identifier's CorrelatedInvocationsID field name to make it match the proto. It was CorrelatedInvocationID before. We need to have CorrelatedInvocationsID in the ToProto method to deduplicate builds that have the same invocation id.
This cl make correction to the identifier's CorrelatedInvocationsID field name to make it match the proto. It was CorrelatedInvocationID before.
We need to have CorrelatedInvocationsID in the ToProto method to deduplicate builds that have the same invocation id.