Skip to content

Introduce generics to splay #346

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

Merged
merged 4 commits into from
Jun 25, 2022
Merged

Conversation

sejongk
Copy link
Contributor

@sejongk sejongk commented Jun 23, 2022

What this PR does / why we need it:

Introduce generics to splay

Which issue(s) this PR fixes:

Fixes #304

Special notes for your reviewer:

By introducing generics to splay, we don't have to use the type assertions in RGATreeSplit and RGATreeList.
If there is anything that needs improvement, please let me know. :)

Does this PR introduce a user-facing change?:


Additional documentation:


Checklist:

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

@CLAassistant
Copy link

CLAassistant commented Jun 23, 2022

CLA assistant check
All committers have signed the CLA.

@hackerwins hackerwins self-requested a review June 24, 2022 00:55
@hackerwins
Copy link
Member

@sejongk Thanks for your contribution.
Please check the lint errors on the CI.

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.

Looking at the lint errors, it is thought to be the lint malfunction. 🤔
I changed the Node to the same receiver name as Tree to avoid it for now.

@codecov
Copy link

codecov bot commented Jun 25, 2022

Codecov Report

Merging #346 (43e44ab) into main (aea0304) will not change coverage.
The diff coverage is 84.21%.

@@           Coverage Diff           @@
##             main     #346   +/-   ##
=======================================
  Coverage   48.30%   48.30%           
=======================================
  Files          65       65           
  Lines        5473     5473           
=======================================
  Hits         2644     2644           
  Misses       2551     2551           
  Partials      278      278           
Impacted Files Coverage Δ
pkg/splay/splay.go 87.95% <80.00%> (ø)
pkg/document/json/rga_tree_list.go 64.28% <100.00%> (ø)
pkg/document/json/rga_tree_split.go 75.00% <100.00%> (ø)

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 aea0304...43e44ab. Read the comment docs.

@hackerwins hackerwins self-requested a review June 25, 2022 03:49
@hackerwins hackerwins merged commit 206a0ab into yorkie-team:main Jun 25, 2022
@sejongk
Copy link
Contributor Author

sejongk commented Jun 25, 2022

@hackerwins Thanks for your edit.

Looking at the lint errors, it is thought to be the lint malfunction.

I totally agree with you. I will check whether the linter is working properly and share related information if the current linter's upgrade is necessary.

@sejongk sejongk deleted the splay-generics branch June 25, 2022 04:14
@sejongk
Copy link
Contributor Author

sejongk commented Jun 25, 2022

@hackerwins It seems a bug in the revive module enabled in golangci-lint, and the bug is fixed (ref. mgechev/revive#692).
When golangci-lint bumps up its revive to the new version that includes the patch, we may resolve the above problem by upgrading golangci-lint.

jeonjonghyeok pushed a commit to jeonjonghyeok/yorkie that referenced this pull request Aug 4, 2022
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.

Update Go version to 1.18
3 participants