working_copy: mark unused conflict_id proto field reserved
#7234
Conversation
The field has been unused since it was deprecated in 97b81a0 (~2 years ago). It should have been marked reserved instead of deprecated already then.
| // Set only if file_type is Conflict | ||
| bytes conflict_id = 4 [deprecated = true]; | ||
| MaterializedConflictData materialized_conflict_data = 5; | ||
| reserved 4; | ||
| } |
There was a problem hiding this comment.
nit: any reason to move this to the bottom? I personally like to keep the fields ordered and have a comment like reserved 4; // old Conflict file_type info or something. Not a big deal, though.
There was a problem hiding this comment.
This is what I usually see in internal proto definitions at Google. I suspect the reason for putting them out of order is that you can include many tags in a single reserved statement (e.g. reserved 3, 14, 15;). I don't know if there's a good reason for grouping them like that, or if there's a good reason to not explain what they used to be used for. I'm happy to do it differently here.
There was a problem hiding this comment.
We have two reserved fields in op_store.proto. The message they're in (View) actually has its fields grouped logically rather than ordered by tag number. We could put the reserved fields close to where they logically belong, but maybe this is another reason to move them to the end so they're less distracting. We can still have a comment about what they used to be for.
The field has been unused since it was deprecated in 97b81a0 (~2 years ago). It should have been marked reserved instead of deprecated already then.
Checklist
If applicable:
CHANGELOG.mdREADME.md,docs/,demos/)cli/src/config-schema.json)