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

Add RHTPQMap and Change CRDTElement type from class to protocol #13

Merged
merged 17 commits into from
Oct 6, 2022
Merged

Add RHTPQMap and Change CRDTElement type from class to protocol #13

merged 17 commits into from
Oct 6, 2022

Conversation

hyeongsik-won
Copy link
Contributor

@hyeongsik-won hyeongsik-won commented Oct 5, 2022 •
edited
Loading

What this PR does / why we need it:

  • Change Heap based on JS-SDK.
  • Add RHTPQMap & Test cases.
  • Change RHTPQMapNode to be the value of HeaNode.
  • Change some member variable names of RHTPQMapNode.
    • strKey -> rhtKey
    • value -> rhtValue
  • Change some method’s names in RHTPQMap(Fix for clear behavior of methods).
    @hackerwins If it's not appropriate, I'll roll back.
    • delete -> remove
    • purge -> delete
    • remove -> delete
    • release -> delete
  • Change CRDTElement type from class to protocol
  • Change setActorID of TimeTicket to mutating and remove the return type.
  • Change TimeTicket/toIDString to private.
  • Change Swift-tools-version to 5.7.
  • Change the access modifier of var and functions of SplayNode.
  • Remove lint errors.
  • Modify compare method of CRDTArray.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Additional documentation:

Checklist:

  • Added relevant tests or not required
  • Didn't break anything

@hyeongsik-won hyeongsik-won added the enhancement 🌟 New feature or request label Oct 5, 2022
@hyeongsik-won hyeongsik-won self-assigned this Oct 5, 2022
hackerwins
hackerwins approved these changes Oct 5, 2022
View reviewed changes
Copy link
Member

@hackerwins hackerwins left a comment

Choose a reason for hiding this comment

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

Overall looks good.
I left a few comments.

Sources/Document/CRDT/CRDTElement.swift Show resolved Hide resolved
Sources/Document/CRDT/RGATreeList.swift Show resolved Hide resolved
7hong13 added a commit to yorkie-team/yorkie-android-sdk that referenced this pull request Oct 5, 2022
@7hong13
Replace 'editedAt' with 'executedAt'
e7b8ded 
this is done based on the following discussion:
yorkie-team/yorkie-ios-sdk#13 (comment)
@7hong13 7hong13 mentioned this pull request Oct 5, 2022
Merged
2 tasks
humdrum
humdrum reviewed Oct 5, 2022
View reviewed changes
Sources/Util/Heap.swift Outdated Show resolved Hide resolved
Sources/Document/CRDT/RHTPQMap.swift Outdated Show resolved Hide resolved
Sources/Document/CRDT/RHTPQMap.swift Outdated Show resolved Hide resolved
humdrum
humdrum reviewed Oct 6, 2022
View reviewed changes
Sources/Util/Heap.swift Outdated Show resolved Hide resolved
humdrum
humdrum reviewed Oct 6, 2022
View reviewed changes
Sources/Util/Heap.swift Outdated Show resolved Hide resolved
humdrum
humdrum reviewed Oct 6, 2022
View reviewed changes
Sources/Util/Heap.swift Outdated Show resolved Hide resolved
humdrum
humdrum reviewed Oct 6, 2022
View reviewed changes
Sources/Util/Heap.swift Outdated Show resolved Hide resolved
@hyeongsik-won
Copy link
Contributor Author

hyeongsik-won commented Oct 6, 2022 •
edited
Loading

@humdrum Added the empty_count rule to SwiftLint. Thank you for the good suggestion.(cdfa29d)

humdrum
humdrum approved these changes Oct 6, 2022
View reviewed changes
Copy link
Contributor

@humdrum humdrum left a comment

Choose a reason for hiding this comment

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

LGTM!

@hyeongsik-won hyeongsik-won merged commit 12487cc into yorkie-team:main Oct 6, 2022
hackerwins added a commit to yorkie-team/yorkie-android-sdk that referenced this pull request Oct 6, 2022
@7hong13 @hackerwins
Implements CrdtArray (#12)
f4f9302 
Some methods are delayed to be implemented until they are actually needed.

This commit replaces 'editedAt' with 'executedAt'. This is done based on the
following discussion:

yorkie-team/yorkie-ios-sdk#13 (comment)

Co-authored-by: Youngteac Hong <susukang98@gmail.com>
7hong13 added a commit to yorkie-team/yorkie-android-sdk that referenced this pull request Oct 7, 2022
@7hong13 @hackerwins
Improve test coverage (#19)
af45086 
* Implement CrdtArray

* Fix comments typo

* Remove unnecessary function

* Implement abstract methods in CrdtElement

some methods are delayed to be implemented until they are actually needed.

* Format fixed

* Add unit test on CrdtArray

* Remove deprecated imports

* Change function names

* Replace 'editedAt' with 'executedAt'

this is done based on the following discussion:
yorkie-team/yorkie-ios-sdk#13 (comment)

* Apply suggestions from code review

* Resolve conflicts

* Resolve conflicts

* Improve test coverage

* Resolve conflicts

* Format fixed

* Change function to property and rollback reverted work

Co-authored-by: Youngteac Hong <susukang98@gmail.com>
@7hong13 7hong13 mentioned this pull request Dec 18, 2022
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@7hong13 7hong13 7hong13 left review comments

@hackerwins hackerwins hackerwins approved these changes

@humdrum humdrum humdrum approved these changes

Assignees

@hyeongsik-won hyeongsik-won

Labels
enhancement 🌟 New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hyeongsik-won @hackerwins @humdrum @7hong13