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.

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

Extract MemPubSub from PubSub #141

Merged
merged 2 commits into from
Jan 9, 2021
Merged

Conversation

hackerwins
Copy link
Member

@hackerwins hackerwins commented Jan 9, 2021

What this PR does / why we need it:

Extract MemPubSub from PubSub

Before implementing the distributed PubSub implementation using ETCD, we
need to extract the temporarily implemented MemoryPubSub and leave only
the high-level abstraction logic in PubSub.

Which issue(s) this PR fixes:

Address #11

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

@hackerwins hackerwins requested a review from dc7303 January 9, 2021 05:30
@hackerwins hackerwins force-pushed the extract-pubsub-interface branch 2 times, most recently from 96fdf4e to 412092f Compare January 9, 2021 05:41
Before implementing the distributed PubSub implementation using ETCD, we
need to extract the temporarily implemented MemoryPubSub and leave only
the high-level abstraction logic in PubSub.
@hackerwins hackerwins force-pushed the extract-pubsub-interface branch from 412092f to d4ca2b4 Compare January 9, 2021 05:47
@codecov
Copy link

codecov bot commented Jan 9, 2021

Codecov Report

Merging #141 (65f0d5e) into master (0b97a53) will increase coverage by 0.06%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #141      +/-   ##
==========================================
+ Coverage   58.52%   58.59%   +0.06%     
==========================================
  Files          27       27              
  Lines        2968     2968              
==========================================
+ Hits         1737     1739       +2     
+ Misses       1057     1056       -1     
+ Partials      174      173       -1     
Impacted Files Coverage Δ
pkg/document/json/rga_tree_split.go 79.55% <0.00%> (+0.88%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b97a53...ec91d69. Read the comment docs.

Copy link
Member

@dc7303 dc7303 left a comment

Choose a reason for hiding this comment

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

That's a great job! Abstraction work is always enjoyable and beneficial to us.
It seems that there will be no problem when introducing a key-value store in the future.

I have given you some opinions, so please check.

yorkie/backend/pubsub/mempubsub/mempubsub.go Outdated Show resolved Hide resolved
yorkie/backend/pubsub/mempubsub/mempubsub.go Outdated Show resolved Hide resolved
Copy link
Member

@dc7303 dc7303 left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thank you for confirming my suggestion.

@dc7303 dc7303 assigned hackerwins and unassigned dc7303 Jan 9, 2021
@hackerwins hackerwins merged commit 1a2ccef into master Jan 9, 2021
@hackerwins hackerwins deleted the extract-pubsub-interface branch January 9, 2021 09:04
jeonjonghyeok pushed a commit to jeonjonghyeok/yorkie that referenced this pull request Aug 4, 2022
Before implementing the distributed PubSub implementation using ETCD, we
need to extract the temporarily implemented MemoryPubSub and leave only
the high-level abstraction logic in PubSub.
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.

2 participants