re: #29906 (review)
Update is a bit of an ambiguous name imo, it doesn’t clearly indicate if it means replacing or merging into the existing result. I think for this commit, where we can only overwrite the existing result, I think Replace would be better. For the next commits in #25665 where we actually add the merging functionality, I think Merge or Combine would be a better choice.
We could definitely have a Replace method in the future. The only reason I didn’t add one now is that so far after updating a lot of code to use the Result class in PR’s #29700 and #25722, I haven’t seen uses for a Replace method. If you think having a Replace() method would good to add here for a future use-case, or for the sake of completeness, I’d be happy to add it. However, I don’t think any changed lines here should be calling Replace() because they are just trying to update result values, not reset and replace the entire result object.
I did originally consider calling this method Merge instead of Update but I liked Update better for 2 reasons. First, I liked consistency with python’s dict.update method which takes fields from two different dict objects and combines them, just like this Update function does. Second, I think the name Merge would be misleading in the majority of cases where individual values are not merged, only different fields are combined, and new field values overwrite old values.
Your suggestion Combine could also work instead of Update but I think is less clear syntactically. For example, I think a.Update(b) maps pretty well to “update object a with contents of object b”, but it’s less obvious what a.Combine(b) would be supposed to do.
