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

add sequencer of mongo #376

Merged
merged 19 commits into from
Jan 21, 2022
Merged

add sequencer of mongo #376

merged 19 commits into from
Jan 21, 2022

Conversation

LXPWing
Copy link
Member

@LXPWing LXPWing commented Dec 26, 2021

What this PR does:
add sequencer of mongo #149

@codecov
Copy link

codecov bot commented Dec 26, 2021

Codecov Report

Merging #376 (9794d2a) into main (d75608f) will increase coverage by 0.45%.
The diff coverage is 51.78%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #376      +/-   ##
==========================================
+ Coverage   54.26%   54.72%   +0.45%     
==========================================
  Files         108      108              
  Lines        5921     5972      +51     
==========================================
+ Hits         3213     3268      +55     
+ Misses       2380     2365      -15     
- Partials      328      339      +11     
Impacted Files Coverage Δ
components/lock/mongo/mongo_lock.go 58.82% <0.00%> (+3.26%) ⬆️
components/pkg/utils/mongo.go 0.00% <0.00%> (ø)
components/sequencer/mongo/mongo_sequencer.go 61.05% <61.05%> (ø)
components/rpc/invoker/mosn/channel/xchannel.go 80.64% <0.00%> (-3.23%) ⬇️
components/pkg/mock/mongo_lock_mock.go

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 d75608f...9794d2a. Read the comment docs.

Copy link
Member

@seeflood seeflood left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!
The binary file demo/configuration/apollo/apolloClientDemo shouldn't be added. You can add it into .gitignore

@seeflood
Copy link
Member

seeflood commented Dec 27, 2021

Could u help add documents for mongodb lock and mongodb sequencer ? You can take consul lock chinese doc and consul lock english doc as examples.

@ZLBer @whalesongAndLittleFish @stulzq Could you help review this PR in detail ? Thanks!

@LXPWing
Copy link
Member Author

LXPWing commented Dec 27, 2021

@seeflood I need to take an exam in the next few days, and I'll be adding documents for mongodb lock and mongodb sequencer over the weekend.

@seeflood
Copy link
Member

@LXPWing Wish you success in your exam ! 青春 is so beautiful 😢

@LXPWing
Copy link
Member Author

LXPWing commented Jan 2, 2022

Hi @seeflood I don't understand why codecov/project is wrong and I should do

@ZLBer
Copy link
Member

ZLBer commented Jan 5, 2022

@seeflood some bugs of codecov?

@seeflood
Copy link
Member

seeflood commented Jan 5, 2022

Sorry I was on a vacation and just came back :)
I think there is some nondeterminacy in xchannel_test.go:
image
We can ignore it and review other code

ZLBer
ZLBer previously approved these changes Jan 8, 2022
Copy link
Member

@ZLBer ZLBer left a comment

Choose a reason for hiding this comment

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

LGTM

@LXPWing
Copy link
Member Author

LXPWing commented Jan 14, 2022

Hi @seeflood Do I need to modify components/rpc/invoker/mosn/channel/xchannel.go ?

@seeflood
Copy link
Member

seeflood commented Jan 14, 2022

@LXPWing About the codecov warning:

  1. Currently your mock code in components/pkg/utils/mongo_sequencer_mock.go is counted in the codecov statistics, which is not necessary ?
    So you can move the mocking code in this file to a new directory like components/pkg/mock,and then add this mock directory in the ignore list in codecov.yml
  2. Besides, you can add some tests for the code affected by this PR,e.g. mongo_sequencer.go or mongo.go

@zhenjunMa
Copy link
Contributor

@LXPWing About the codecov warning:

  1. Currently your mock code in components/pkg/utils/mongo_sequencer_mock.go is counted in the codecov statistics, which is not necessary ?
    So you can move the mocking code in this file to a new directory like components/pkg/mock,and then add this mock directory in the ignore list in codecov.yml
  2. Besides, you can add some tests for the code affected by this PR,e.g. mongo_sequencer.go or mongo.go

@LXPWing I have reviewed this PR, please fix the codecov problem as above, then we will merged, thanks.

Copy link
Contributor

@zhenjunMa zhenjunMa left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Copy link
Member

@seeflood seeflood left a comment

Choose a reason for hiding this comment

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

lgtm

@seeflood seeflood merged commit 2538341 into mosn:main Jan 21, 2022
@seeflood
Copy link
Member

Thanks for your contribution !

seeflood pushed a commit to seeflood/layotto that referenced this pull request Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants