Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Patch Envelope status spelling error and add ZiOpaqueAppInstanceStatus #29

Merged
merged 2 commits into from
Oct 6, 2023

Conversation

uncleDecart
Copy link
Member

We need ZiOpaqueAppInstanceStatus in order to distinguish between status send by app and information about patch envelope assigned to app instance

…tatus

We need ZiOpaqueAppInstanceStatus in order to distinguish between status
send by app and information about patch envelope assigned to app instance

Signed-off-by: Pavel Abramov <uncle.decart@gmail.com>
@uncleDecart
Copy link
Member Author

@eriknordmark can you please check?Renaming is a breaking change, but status handling has not yet been merged to eve, so it does not affect it.

Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is not used by any code we know of in EVE or controllers, fixing the typo in PATCH_RECIEVED is not an issue even at recompile of that software.
And since we are not changing the value it would never be an issue in the protobuf wire encoding compatibility.

Ignoring the yetus complaints about the prefixes since we follow the patten for the existing enum values.

@eriknordmark
Copy link
Contributor

Looking at the build and test failure, the diffs are additional empty lines in the output from protoc. Are you building this using the build container (which runs a well-know version of protoc)?
In any case we'll merge it, but want to understand why we get false alarms like this.

Signed-off-by: Pavel Abramov <uncle.decart@gmail.com>
@uncleDecart
Copy link
Member Author

@eriknordmark I rebuild it using devcontainer, works like charm :)

@eriknordmark eriknordmark merged commit 07e6652 into lf-edge:main Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants