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

chore: code fixing, remove redundant and add missing #880

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

chore: code fixing, remove redundant and add missing #880

wants to merge 1 commit into from

Conversation

lxl66566
Copy link
Collaborator

@lxl66566 lxl66566 commented Jul 11, 2024 •
edited
Loading

Please briefly answer these questions:

  • what problem are you trying to solve? (or if there's no problem, what's the motivation for this change?)

    • redundant .field in KvClient and LeaseClient

  • what changes does this pull request make?

    1. remove redundant .field in KvClient and LeaseClient
    2. add a missing test case for quorum_should_work() test, corresponding to the above node(10).
    3. (trivial) change for loop to sum() in raw_curp/log.rs

  • are there any non-obvious implications of these changes? (does it break compatibility with previous versions, etc)

    • no

@mergify mergify bot requested a review from a team July 11, 2024 06:18
@codecov Codecov
Copy link

codecov bot commented Jul 11, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 94.73684% with 1 line in your changes missing coverage. Please review.

Project coverage is 76.15%. Comparing base (e35b35a) to head (67ba153).
Report is 302 commits behind head on master.

Files with missing lines Patch % Lines
crates/xline/src/storage/kv_store.rs 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #880      +/-   ##
==========================================
+ Coverage   75.55%   76.15%   +0.59%     
==========================================
  Files         180      190      +10     
  Lines       26938    28843    +1905     
  Branches    26938    28843    +1905     
==========================================
+ Hits        20353    21964    +1611     
- Misses       5366     5551     +185     
- Partials     1219     1328     +109

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@CrystalAnalyst
Copy link
Contributor

@bsbds Please take at a look.

Phoenix500526
Phoenix500526 reviewed Jul 25, 2024
View reviewed changes
Copy link
Collaborator

@Phoenix500526 Phoenix500526 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM. Please initiate a pr to the xline-proto repo and squash these commits

@mergify mergify bot requested a review from a team July 25, 2024 02:06
@lxl66566
Copy link
Collaborator Author

rebased. and proto has been updated in #903 .

@mergify Mergify
Copy link

mergify bot commented Jul 25, 2024

@lxl66566 Convert your pr to draft since CI failed

@mergify mergify bot marked this pull request as draft July 25, 2024 07:53
@mergify mergify bot added the CI:fail CI has failed label Jul 25, 2024
@mergify mergify bot marked this pull request as ready for review July 25, 2024 08:28
@mergify mergify bot removed the CI:fail CI has failed label Jul 25, 2024
@mergify Mergify
Copy link

mergify bot commented Jul 29, 2024

@lxl66566 Convert your pr to draft since CI failed

@mergify mergify bot marked this pull request as draft July 29, 2024 16:08
@mergify mergify bot added the CI:fail CI has failed label Jul 29, 2024
@mergify mergify bot marked this pull request as ready for review July 29, 2024 16:41
@mergify mergify bot removed the CI:fail CI has failed label Jul 29, 2024
@lxl66566 lxl66566 force-pushed the code-fix branch 3 times, most recently from 15b13bf to 67ba153 Compare August 2, 2024 03:06
@mergify Mergify
Copy link

mergify bot commented Aug 2, 2024

@lxl66566 Convert your pr to draft since CI failed

@mergify mergify bot marked this pull request as draft August 2, 2024 03:16
@mergify mergify bot added the CI:fail CI has failed label Aug 2, 2024
@mergify mergify bot marked this pull request as ready for review August 2, 2024 03:46
@mergify mergify bot removed the CI:fail CI has failed label Aug 2, 2024
@mergify Mergify
Copy link

mergify bot commented Aug 15, 2024

@lxl66566 Your PR is in conflict and cannot be merged.

@github-actions GitHub Actions
Copy link

This PR is stale because it has been open for 30 days with no activity. Remove stale label or comment or this will be closed in 14 days

@github-actions github-actions bot added Stale and removed Stale labels Sep 15, 2024
@mergify Mergify
Copy link

mergify bot commented Sep 19, 2024

@lxl66566 Convert your pr to draft since CI failed

@mergify mergify bot marked this pull request as draft September 19, 2024 13:09
@mergify mergify bot added the CI:fail CI has failed label Sep 19, 2024
@lxl66566
fix(client): remove redundant, add missing and other trivial change
125d828 
Signed-off-by: lxl66566 <lxl66566@gmail.com>

chore(curp): add a missing testcase to lib.rs

Signed-off-by: lxl66566 <lxl66566@gmail.com>

chore(raw_curp): trivial change to functional expression

Signed-off-by: lxl66566 <lxl66566@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Phoenix500526 Phoenix500526 Phoenix500526 left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
CI:fail CI has failed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lxl66566 @CrystalAnalyst @Phoenix500526