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

Add support for mapping messages to CloudEvents #216

Conversation

sophokles73
Copy link
Contributor

Added support for mapping UMessages to/from CloudEvents using Protobuf format.

Addresses #203

@sophokles73 sophokles73 added the enhancement New feature or request label Nov 27, 2024
@sophokles73 sophokles73 added this to the Spec level 1.6.0.alpha.4 milestone Nov 27, 2024
Copy link
Contributor

@AnotherDaniel AnotherDaniel left a comment

Choose a reason for hiding this comment

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

One question, about copying the cloudevents.proto definition into this repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really want to pull this in as a copy? Or maybe revive the download&compile functionality we originally used in build.rs for external proto files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would rather keep it in our source tree. It is a tagged version (i.e. immutable) and only a single file ...

Copy link
Contributor

Choose a reason for hiding this comment

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

Mh. I'll defer to your judgement on this :) Would it make sense to mention this in NOTICE.md?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, will do.

src/cloudevents.rs Show resolved Hide resolved
@sophokles73 sophokles73 force-pushed the add_support_for_protobuf_cloudevents branch from 8d38738 to 50089cf Compare November 27, 2024 13:22
@sophokles73
Copy link
Contributor Author

@AnotherDaniel would you mind taking another look?

Added support for mapping UMessages to/from CloudEvents using
Protobuf format.
@sophokles73 sophokles73 force-pushed the add_support_for_protobuf_cloudevents branch from 50089cf to 5e22fff Compare November 27, 2024 15:06
@sophokles73
Copy link
Contributor Author

@AnotherDaniel I have updated the NOTICE.md file accordingly.

Copy link
Contributor

@AnotherDaniel AnotherDaniel left a comment

Choose a reason for hiding this comment

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

LGTM

@sophokles73 sophokles73 merged commit db98b70 into eclipse-uprotocol:main Nov 27, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants