Skip to content

Refactor to Facilitate Decoupling from Concrete Implementations of EventFormat #539

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.

Sign up for GitHub

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

Jump to bottom
Merged
pierDipi merged 8 commits into from
Apr 21, 2023
Merged

Refactor to Facilitate Decoupling from Concrete Implementations of EventFormat #539

pierDipi merged 8 commits into from
Apr 21, 2023

Conversation

@skepticoitusInteruptus
Copy link
Contributor

@skepticoitusInteruptus skepticoitusInteruptus commented Mar 23, 2023

Closes #537

Example Usage of Proposed Refactor

import io.cloudevents.CloudEvent;
import io.cloudevents.core.format.EventFormatProvider;
import io.cloudevents.core.builder.CloudEventBuilder;
import io.cloudevents.core.format.ContentType; // New constant to facilitate loose coupling in client applications

CloudEvent event = ...;

byte[] serialized = EventFormatProvider
    .getInstance()
    .resolveFormat(ContentType.PROTO) // EventFormatProvider.resolveFormat(...) overloaded 
    .serialize(event);                // for string and enum input options

@skepticoitusInteruptus skepticoitusInteruptus force-pushed the 537-refactor-facilitate-decoupling-from-eventformat-impl branch from 8f82cdb to 02b5bd1 Compare March 23, 2023 15:27
JemDay
JemDay suggested changes Mar 23, 2023
View reviewed changes
Copy link
Contributor

@JemDay JemDay left a comment

Choose a reason for hiding this comment

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

It looks like you need to sign your 'git commit' to get the build to run.

I do like how this cleans up the dependencies, may need to tweak the documentation on the ContentType to make it clear it's only enumerating the types defined by the various CloudEvent formats.

@JemDay
Copy link
Contributor

JemDay commented Mar 23, 2023

And it looks like you 'pushed' while I was commenting :-)

@skepticoitusInteruptus
Copy link
Contributor Author

skepticoitusInteruptus commented Mar 23, 2023

And it looks like you 'pushed' while I was commenting :-)

Yeah. I didn't intend to push so many commits. I tried (in vain) to rebase all but one of them away. But I ended up with even more than I started out with.

I suck at git. Haha!

pierDipi
pierDipi reviewed Mar 30, 2023
View reviewed changes
@skepticoitusInteruptus skepticoitusInteruptus force-pushed the 537-refactor-facilitate-decoupling-from-eventformat-impl branch from 53e6081 to e1dbe9c Compare April 1, 2023 17:30
pierDipi
pierDipi reviewed Apr 11, 2023
View reviewed changes
pierDipi
pierDipi approved these changes Apr 13, 2023
View reviewed changes
Copy link
Member

@pierDipi pierDipi left a comment

Choose a reason for hiding this comment

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

Thanks!

LGTM

@pierDipi pierDipi merged commit 4ebeab0 into cloudevents:main Apr 21, 2023
alexec pushed a commit to alexec/sdk-java that referenced this pull request Jun 6, 2023
@skepticoitusInteruptus @alexec
Refactor to Facilitate Decoupling from Concrete Implementations of Ev…
9e0a4c7 
…entFormat (cloudevents#539)

- Introduce ContentType enum
- Resolve formats by using the ContentType enum

Signed-off-by: Randi Sheaffer-Klass <[email protected]>
Signed-off-by: Alex Collins <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pierDipi pierDipi pierDipi approved these changes

+1 more reviewer

@JemDay JemDay JemDay requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove Clients' Dependency on Concrete Implementations of EventFormat

3 participants

@skepticoitusInteruptus @JemDay @pierDipi