Skip to content

Adding helpers to Config, PubSubMessage, Metadata. Coverage improved #6

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
merged 5 commits into from
Oct 4, 2017
Merged

Adding helpers to Config, PubSubMessage, Metadata. Coverage improved #6

merged 5 commits into from
Oct 4, 2017

Conversation

akshaisarma
Copy link
Member

No description provided.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 99.426% when pulling 356057d on more-helpers into c9f3f60 on master.

@akshaisarma akshaisarma requested a review from hbxie October 4, 2017 01:55
@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 99.426% when pulling 37d69ab on more-helpers into c9f3f60 on master.

hbxie
hbxie reviewed Oct 4, 2017
View reviewed changes
src/main/java/com/yahoo/bullet/pubsub/Metadata.java
@Getter @Setter
private Signal signal;
private Serializable content;
private Object content;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you change from Serializable to Object and do the type cast after?

Copy link
Member Author

Choose a reason for hiding this comment

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

If the type is Serializable, GSON cannot create an instance of it. So I kept the interface the same (allowing only Serializable objects in or out) but store it as an object. The casting is only done on the getter. GSON is used in PubSubMessage#fromJSON. See a better way?

Copy link
Contributor

Choose a reason for hiding this comment

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

Then add a comment here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do.

hbxie
hbxie previously approved these changes Oct 4, 2017
View reviewed changes
src/main/java/com/yahoo/bullet/pubsub/Metadata.java
@Getter @Setter
private Signal signal;
private Serializable content;
private Object content;
Copy link
Contributor

Choose a reason for hiding this comment

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

Then add a comment here?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 99.426% when pulling 820c024 on more-helpers into c9f3f60 on master.

hbxie
hbxie previously approved these changes Oct 4, 2017
View reviewed changes
@akshaisarma akshaisarma merged commit 1e3f01a into master Oct 4, 2017
@akshaisarma akshaisarma deleted the more-helpers branch October 4, 2017 05:18
@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 99.426% when pulling d155315 on more-helpers into c9f3f60 on master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@hbxie hbxie hbxie left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@hbxie hbxie

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@akshaisarma @coveralls @hbxie