added versioning for migration #1280
Conversation
Signed-off-by: hillium <[email protected]>
| @@ -876,7 +876,17 @@ message DeleteSpansOfFile { | |||
| uint64 whole_file_length = 3; | |||
| } | |||
|
|
|||
| enum MigrationVersion { | |||
| // The initial version. Somehow "UNKNOWN". | |||
| M0 = 0; | |||
There was a problem hiding this comment.
nit, maybe V0 and V1? usually use V as version,
There was a problem hiding this comment.
protocol buffer requires the enum name is unique among all enums in the same package. The variant V1 was used by MetaFileVersion...
| message Migration { | ||
| string creator = 5; |
There was a problem hiding this comment.
nit, maybe sort in the numeric order, put them to the end of the message.
There was a problem hiding this comment.
I think the version and creator field should be more primitive and essential than other fields. So I put them in the first line and people may notice them easier.
|
@Tristan1900: adding LGTM is restricted to approvers and reviewers in OWNERS files. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 3pointer, BornChanger, Tristan1900 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
This PR added the vesrion and creator field to the
Migrationtype for troubleshoting.