Skip to content

Ensure that HubSpot style will pass rosetta annotations into Immutables #55

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

Merged
merged 2 commits into from
Jun 10, 2024

Conversation

suruuK
Copy link
Contributor

@suruuK suruuK commented Jun 10, 2024

This PR ensures that Rosetta annotations get passes along into the immtable object definitions and resolves when we define an immutable like

@Immutable
@HubSpotStyle
@RosettaNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)
public interface ObjectWithBooleanTestIF {
  @JsonSetter("someValue")
  @JsonProperty("isSomeValue")
  @RosettaProperty("is_some_value")
  Optional<Boolean> isSomeValue();
}

Rosetta will serialize to {'is_some_value':true} but deserialization will only happen wen defining {'someValue':true} sicne the RosettaProperty is not passed into the generated immutable class.

cc @stevie400 @Xcelled @jhaber @jaredstehler

Copy link
Contributor

@jaredstehler jaredstehler left a comment

Choose a reason for hiding this comment

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

🚢

@@ -62,7 +62,6 @@
<dependency>
<groupId>com.hubspot.rosetta</groupId>
<artifactId>RosettaAnnotations</artifactId>
<scope>test</scope>
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit of an unfortunate dependency to pick up, do you think this can go away at some point in the future or are we stuck with it forever?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we can get away with a @Inherited tag on the RosettaAnnotation, trying that out now....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, that did not work. looks like its unavoidable to include @RosettaAnnotation in hubspot-immutables or something like @ImmutableInherited (from hubspot-immutables) into Rosetta. We could seperate out the RosettaAnnotation annotation into a seperate RosettaAnnotationsBase module, to cut down on the dependency size? 🤷

Copy link
Member

@jhaber jhaber Jun 10, 2024

Choose a reason for hiding this comment

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

I'm less worried about the size of the dep tree, moreso just feels weird to take a hard dependency on Rosetta here. But if it can't be avoided I guess we can just go with it

@suruuK suruuK merged commit e580347 into master Jun 10, 2024
1 check failed
@suruuK suruuK deleted the pass_rosetta_annotations branch June 10, 2024 19:46
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.

3 participants