You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Requesting changes: I would want some comments to actually be addressed instead of being argued as do not actually need to change (or if the argument of not changing is good, I would like to make sure I review the reply). After seeing the answers, I believe we need at least:
use builders for arguments when the argument count starts being large (usually after around 4-5 arguments ... and always if there are same-type arguments especially bools or nullable things)
do not use operator= to mean anything else than "exact copy"
As an offline discussion, it was discussed to make these as a separate PR instead of in #33991. I am ok with that as long as there is commitment for the followup to happen soon. Because historically we had a very low rate of actual promised followups, I would like to have a commitment that one of 2 things happen:
followup PR(s) fix the above (great!)
followups do not happen in 2-3 weeks, in which case Implementation of service area server #33991 and any followups on it would get rolled back (this should really not happen ... just having this as a guarantee that the followups have high priority)
Requesting changes: I would want some comments to actually be addressed instead of being argued as
do not actually need to change
(or if the argument of not changing is good, I would like to make sure I review the reply). After seeing the answers, I believe we need at least:Originally posted by @andy31415 in #33991 (review)
The text was updated successfully, but these errors were encountered: