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

Add account.historyCount RPC #265

Merged
merged 13 commits into from
May 26, 2021
Merged

Add account.historyCount RPC #265

merged 13 commits into from
May 26, 2021

Conversation

siradji
Copy link
Contributor

@siradji siradji commented May 20, 2021

What kind of PR is this?:

/kind feature

What this PR does / why we need it:

RPC implementation #48

Which issue(s) does this PR fixes?:

Fixes #

Additional comments?:

@netlify
Copy link

netlify bot commented May 20, 2021 •
edited
Loading

Deploy Preview for jellyfish-defi ready!

Built with commit f63df6e

https://deploy-preview-265--jellyfish-defi.netlify.app

@github-actions
Copy link

github-actions bot commented May 20, 2021 •
edited
Loading

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
packages/jellyfish/dist/jellyfish.umd.js 19.44 KB (+1.49% 🔺) 389 ms (+1.49% 🔺) 156 ms (+1.94% 🔺) 545 ms

@codecov
Copy link

codecov bot commented May 20, 2021 •
edited
Loading

Codecov Report

Merging #265 (5485870) into main (838a2a8) will increase coverage by 0.39%.
The diff coverage is 100.00%.

❗ Current head 5485870 differs from pull request most recent head 730a308. Consider uploading reports for the commit 730a308 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #265      +/-   ##
==========================================
+ Coverage   96.39%   96.78%   +0.39%     
==========================================
  Files          82       88       +6     
  Lines        2161     2366     +205     
  Branches      277      303      +26     
==========================================
+ Hits         2083     2290     +207     
+ Misses         78       76       -2
Impacted Files Coverage Δ
...ish-api-core/__tests__/container_adapter_client.ts 100.00% <100.00%> (ø)
...ackages/jellyfish-api-core/src/category/account.ts 100.00% <100.00%> (ø)
...ages/jellyfish-api-core/src/category/blockchain.ts 100.00% <100.00%> (ø)
packages/jellyfish-api-core/src/category/wallet.ts 100.00% <100.00%> (ø)
packages/jellyfish-api-core/src/index.ts 100.00% <100.00%> (ø)
packages/jellyfish-api-jsonrpc/src/index.ts 97.43% <100.00%> (ø)
...ackages/jellyfish-transaction-builder/src/index.ts 100.00% <100.00%> (ø)
...transaction-builder/src/txn/txn_builder_account.ts 100.00% <100.00%> (ø)
...h-transaction-builder/src/txn/txn_builder_error.ts 100.00% <100.00%> (ø)
...ransaction-builder/src/txn/txn_builder_liq_pool.ts 100.00% <100.00%> (ø)
... and 16 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 838a2a8...730a308. Read the comment docs.

canonbrother
canonbrother requested changes May 20, 2021
View reviewed changes
Copy link
Contributor

@canonbrother canonbrother left a comment

Choose a reason for hiding this comment

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

overall looks great
just the alignment on the test

packages/jellyfish-api-core/__tests__/category/account.test.ts Show resolved Hide resolved
canonbrother
canonbrother previously approved these changes May 20, 2021
View reviewed changes
@thedoublejay thedoublejay changed the title accountHistoryCount RCP implementation accountHistoryCount RPC implementation May 21, 2021
@siradji siradji marked this pull request as ready for review May 21, 2021 07:39
@codeclimate
Copy link

codeclimate bot commented May 21, 2021 •
edited
Loading

Code Climate has analyzed commit 730a308 and detected 0 issues on this pull request.

View more on Code Climate.

canonbrother
canonbrother previously approved these changes May 21, 2021
View reviewed changes
@fuxingloh fuxingloh changed the title accountHistoryCount RPC implementation add account.historyCount rpc May 21, 2021
@fuxingloh fuxingloh marked this pull request as draft May 21, 2021 13:31
@fuxingloh fuxingloh mentioned this pull request May 23, 2021
Open
@siradji siradji marked this pull request as ready for review May 24, 2021 14:56
fuxingloh
fuxingloh requested changes May 24, 2021
View reviewed changes
Copy link
Member

@fuxingloh fuxingloh left a comment

Choose a reason for hiding this comment

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

a few changes to take note of

website/docs/jellyfish/api/account.md Outdated Show resolved Hide resolved
website/docs/jellyfish/api/account.md Outdated Show resolved Hide resolved
packages/jellyfish-api-core/src/category/account.ts Outdated Show resolved Hide resolved
packages/jellyfish-api-core/src/category/account.ts Outdated Show resolved Hide resolved
packages/jellyfish-api-core/src/category/account.ts Outdated Show resolved Hide resolved
packages/jellyfish-api-core/src/category/account.ts Show resolved Hide resolved
@fuxingloh fuxingloh marked this pull request as draft May 24, 2021 15:44
siradji and others added 6 commits May 24, 2021 16:52
@siradji @fuxingloh
Update website/docs/jellyfish/api/account.md
9ae788c 
Suggested Changes by @fuxingloh

Co-authored-by: Fuxing Loh <4266087+fuxingloh@users.noreply.github.com>
@siradji @fuxingloh
Update packages/jellyfish-api-core/src/category/account.ts
a49021a 
Co-authored-by: Fuxing Loh <4266087+fuxingloh@users.noreply.github.com>
@siradji @fuxingloh
Update packages/jellyfish-api-core/src/category/account.ts
c8a5171 
Co-authored-by: Fuxing Loh <4266087+fuxingloh@users.noreply.github.com>
@siradji @fuxingloh
Update website/docs/jellyfish/api/account.md
56fcb74 
Co-authored-by: Fuxing Loh <4266087+fuxingloh@users.noreply.github.com>
@siradji @fuxingloh
Update packages/jellyfish-api-core/src/category/account.ts
689817d 
Co-authored-by: Fuxing Loh <4266087+fuxingloh@users.noreply.github.com>
@siradji @fuxingloh
Update packages/jellyfish-api-core/src/category/account.ts
f63df6e 
Co-authored-by: Fuxing Loh <4266087+fuxingloh@users.noreply.github.com>
@fuxingloh fuxingloh changed the title add account.historyCount rpc Add account.historyCount RPC May 24, 2021
@siradji siradji marked this pull request as ready for review May 26, 2021 07:47
@fuxingloh fuxingloh enabled auto-merge (squash) May 26, 2021 11:43
@fuxingloh fuxingloh disabled auto-merge May 26, 2021 14:48
@fuxingloh fuxingloh merged commit 7f1e36d into main May 26, 2021
@fuxingloh fuxingloh deleted the siradji/account-accounthistorycount branch May 26, 2021 14:48
@siradji siradji mentioned this pull request May 31, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fuxingloh fuxingloh fuxingloh approved these changes

@jingyi2811 jingyi2811 jingyi2811 approved these changes

@canonbrother canonbrother Awaiting requested review from canonbrother canonbrother is a code owner

@thedoublejay thedoublejay Awaiting requested review from thedoublejay

Assignees
No one assigned
Labels
kind/feature New feature request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@siradji @fuxingloh @jingyi2811 @canonbrother @thedoublejay @defichain-bot