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.

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

feature/serialization helpers #767

Merged
merged 5 commits into from
Nov 2, 2023
Merged

feature/serialization helpers #767

merged 5 commits into from
Nov 2, 2023

Conversation

baywet
Copy link
Member

@baywet baywet commented Oct 31, 2023

@baywet baywet self-assigned this Oct 31, 2023
@baywet baywet enabled auto-merge October 31, 2023 21:09
@sonarqubecloud SonarQubeCloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarqubecloud SonarQubeCloud
Copy link

Please retry analysis of this Pull-Request directly on SonarCloud.

@sonarqubecloud SonarQubeCloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarqubecloud SonarQubeCloud
Copy link

Please retry analysis of this Pull-Request directly on SonarCloud.

@sonarqubecloud SonarQubeCloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarqubecloud SonarQubeCloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

ramsessanchez
ramsessanchez reviewed Nov 1, 2023
View reviewed changes
Copy link
Contributor

@ramsessanchez ramsessanchez left a comment

Choose a reason for hiding this comment

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

@baywet , I'm wondering if the serializeAsString methods should be differentiated similar as to how you differentiate the deserialize methods. Perhaps serializeCollectionAsString or serializeCollectionAsStream? Otherwise looks good!

@baywet
Copy link
Member Author

baywet commented Nov 2, 2023

@ramsessanchez yeah I was wondering that too, we don't technically need to do it as the parameters types are different. It's a matter of consistency vs convenience. I believe the current situation feels familiar to the ecosystem (i.e. Java devs are used to overload with different parameters types)

@baywet baywet merged commit 7490835 into main Nov 2, 2023
10 checks passed
@baywet baywet deleted the feature/serialization-helpers branch November 2, 2023 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ramsessanchez ramsessanchez ramsessanchez approved these changes

@MIchaelMainer MIchaelMainer Awaiting requested review from MIchaelMainer

@zengin zengin Awaiting requested review from zengin

@andrueastman andrueastman Awaiting requested review from andrueastman

Assignees

@baywet baywet

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@baywet @ramsessanchez