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

Refactor MessageExtras #595

Merged
merged 5 commits into from Nov 23, 2020
Merged

Refactor MessageExtras #595

merged 5 commits into from Nov 23, 2020

Conversation

sacOO7
Copy link
Collaborator

@sacOO7 sacOO7 commented Aug 23, 2020 •
edited

@sacOO7 sacOO7 changed the title Refactor MessageExtras [WIP] Refactor MessageExtras Aug 23, 2020
@sacOO7
Copy link
Collaborator Author

sacOO7 commented Aug 23, 2020 •
edited

Refactored based on the following assumptions

  • DeltaExtras is set by the server, the client cannot modify it.
  • MessageExtras jsonObject can not be set as null.

@sacOO7 sacOO7 changed the title [WIP] Refactor MessageExtras Refactor MessageExtras Sep 2, 2020
@QuintinWillison
Copy link
Contributor

Hi @sacOO7 ... looks like you need to rebase against main. And, also, I thought we had discussed in Slack the removal of the third-party (Jetbrains) annotations. Are you planning to update this PR to remove those or do you need me to take it over?

@sacOO7
Copy link
Collaborator Author

sacOO7 commented Oct 1, 2020

Yes, I will remove the dependency and fix conflicts

@sacOO7
Removed jetbrains annotations lib, removed null annotations from the …
1f9da19 
…code
@sacOO7
Added checkstyle formatting to the code
4481cb0
@sacOO7
Merge branch 'main' into feature/580-refactor-message-extras
4ac5353 
# Conflicts:
#	lib/src/main/java/io/ably/lib/types/BaseMessage.java
#	lib/src/main/java/io/ably/lib/types/DeltaExtras.java
#	lib/src/main/java/io/ably/lib/types/Message.java
#	lib/src/main/java/io/ably/lib/types/MessageExtras.java
#	lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java
@sacOO7
Copy link
Collaborator Author

sacOO7 commented Nov 16, 2020

@vzhikserg in case you need more context, link to related convo - https://ably-real-time.slack.com/archives/C019LDWM9UH/p1603817789008500

vzhikserg
vzhikserg approved these changes Nov 22, 2020
View reviewed changes
Copy link
Contributor

@vzhikserg vzhikserg left a comment

Choose a reason for hiding this comment

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

This PR looks fine taking into account that deltas shouldn't be written/serialized by the client and if it comes from the server side it is a part of the json object.

@QuintinWillison QuintinWillison merged commit 9621342 into main Nov 23, 2020
@QuintinWillison QuintinWillison deleted the feature/580-refactor-message-extras branch November 23, 2020 16:36
@QuintinWillison QuintinWillison mentioned this pull request Dec 22, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vzhikserg vzhikserg vzhikserg approved these changes

@QuintinWillison QuintinWillison Awaiting requested review from QuintinWillison

Assignees

@sacOO7 sacOO7

Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Address impact of change to interface on extras field on Message
3 participants
@sacOO7 @QuintinWillison @vzhikserg