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

Change how chain links are stored #489

Merged
merged 11 commits into from
Jun 18, 2021
Merged

Conversation

dadamu
Copy link
Contributor

@dadamu dadamu commented Jun 15, 2021

Description

This PR changes how chain links are stored and allows to query paginated chain links.
Close #480

Checklist

  • Targeted PR against correct branch.
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Wrote unit tests.
  • Wrote integration tests (simulation & CLI).
  • Updated the documentation.
  • Added an entry to the CHANGELOG.md file.
  • Re-reviewed Files changed in the Github PR explorer.

@dadamu dadamu changed the title Paul/change chain links stored Change how chain links are stored Jun 15, 2021
@codecov
Copy link

codecov bot commented Jun 15, 2021

Codecov Report

Merging #489 (03f2291) into master (578f794) will increase coverage by 0.25%.
The diff coverage is 78.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #489      +/-   ##
==========================================
+ Coverage   83.74%   83.99%   +0.25%     
==========================================
  Files          91       91              
  Lines        4127     4124       -3     
==========================================
+ Hits         3456     3464       +8     
+ Misses        513      500      -13     
- Partials      158      160       +2     
Impacted Files Coverage Δ
x/profiles/client/cli/query.go 0.00% <0.00%> (ø)
x/profiles/keeper/keeper.go 86.66% <0.00%> (+5.18%) ⬆️
x/profiles/types/account.go 54.19% <ø> (+2.79%) ⬆️
x/profiles/keeper/alias_functions.go 78.00% <61.11%> (-9.50%) ⬇️
x/profiles/client/cli/cli_chain_links.go 87.01% <77.77%> (-4.77%) ⬇️
x/profiles/keeper/grpc_query.go 75.00% <90.00%> (-1.20%) ⬇️
x/profiles/keeper/genesis.go 81.81% <100.00%> (+3.43%) ⬆️
x/profiles/keeper/keeper_chain_links.go 90.90% <100.00%> (+3.40%) ⬆️
x/profiles/keeper/msg_server_chain_link.go 100.00% <100.00%> (ø)
x/profiles/keeper/relay_chain_links.go 86.66% <100.00%> (ø)
... and 3 more

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 578f794...03f2291. Read the comment docs.

@dadamu dadamu marked this pull request as ready for review June 15, 2021 11:49
@RiccardoM
Copy link
Contributor

Instead of defining a ChainLinkEntry, what I think we can do is simply add the User field to the ChainLink message itself. This should make it more easy to manage, export and import.

Copy link
Contributor

@leobragaz leobragaz left a comment

Choose a reason for hiding this comment

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

Looks good to me, only a thing that IMO can be improved

Copy link
Contributor

@RiccardoM RiccardoM left a comment

Choose a reason for hiding this comment

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

Just a couple of minor improvements

x/profiles/keeper/alias_functions.go Outdated Show resolved Hide resolved
x/profiles/keeper/alias_functions.go Outdated Show resolved Hide resolved
@RiccardoM RiccardoM enabled auto-merge (squash) June 18, 2021 05:47
@RiccardoM RiccardoM merged commit 251e8dc into master Jun 18, 2021
@RiccardoM RiccardoM deleted the paul/change-chain-links-stored branch June 18, 2021 05:47
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.

Change how chain links are stored
3 participants