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 comparison analysis table for supported tool #427

Merged
merged 3 commits into from
Aug 10, 2022

Conversation

natanasow
Copy link
Collaborator

Signed-off-by: nikolay n.atanasow94@gmail.com

Description:

Related issue(s):

Fixes #380

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

Signed-off-by: nikolay <n.atanasow94@gmail.com>
@natanasow natanasow self-assigned this Aug 8, 2022
@codecov-commenter
Copy link

codecov-commenter commented Aug 8, 2022

Codecov Report

Merging #427 (51ddf20) into main (06e89ee) will increase coverage by 2.57%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #427      +/-   ##
==========================================
+ Coverage   73.65%   76.23%   +2.57%     
==========================================
  Files          10       11       +1     
  Lines         835      913      +78     
  Branches      137      144       +7     
==========================================
+ Hits          615      696      +81     
+ Misses        172      165       -7     
- Partials       48       52       +4     
Impacted Files Coverage Δ
packages/relay/src/formatters.ts 66.66% <0.00%> (ø)
packages/relay/src/lib/clients/sdkClient.ts 8.18% <0.00%> (+0.63%) ⬆️
packages/relay/src/lib/precheck.ts 90.14% <0.00%> (+0.74%) ⬆️
packages/relay/src/lib/eth.ts 84.24% <0.00%> (+2.99%) ⬆️
packages/relay/src/lib/clients/mirrorNodeClient.ts 90.14% <0.00%> (+4.11%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Ivo-Yankov
Ivo-Yankov previously approved these changes Aug 8, 2022
@Nana-EC Nana-EC added documentation Improvements or additions to documentation enhancement New feature or request limechain P2 labels Aug 8, 2022
@Nana-EC Nana-EC added this to the 0.6.0 milestone Aug 8, 2022
Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

Looking good.
Suggested additions

tools/README.md Outdated
| Contract Deployment | ✅ | ✅ | ✅ | ✅ |
| Can use the contract instance after deploy without re-initialization | ✅ | ✅ | ❌ | ❌ |
| Contract Method Call | ✅ | ✅ | ✅ | ✅ |
| Contract Method Call Execution | ✅ | ✅ | ✅ | ✅ |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a note under the table highlighted the re-initialization issue.
You can also note the ethersjs bug so that there's transparency to the community.

@@ -0,0 +1,8 @@
### Supported tools
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add the following sentence
"The JSON RPC relay serves as an interface to the Hedera network for ethereum developer tools that utilize the implemented JSON RPC APIs
The following development tools have been tested and the extent of their coverage is noted below"

tools/README.md Outdated
| | web3js | Truffle | ethers | Hardhat |
|----------------------------------------------------------------------|--------|---------|--------|---------|
| Transfer HBARS | ✅ | ✅ | ✅ | ✅ |
| Contract Deployment | ✅ | ✅ | ✅ | ✅ |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
| Contract Deployment |||||
| Contract Deployment |||||

tools/README.md Outdated
Comment on lines 7 to 8
| Contract Method Call | ✅ | ✅ | ✅ | ✅ |
| Contract Method Call Execution | ✅ | ✅ | ✅ | ✅ |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
| Contract Method Call |||||
| Contract Method Call Execution |||||
| Contract View Function Call |||||
| Contract Function Call |||||

Signed-off-by: nikolay <n.atanasow94@gmail.com>
Nana-EC
Nana-EC previously approved these changes Aug 9, 2022
Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

LG, if you circle back you can move the sentence up

tools/README.md Outdated
Note: On contract deployment, most of the tools (e.g. [ethers](https://docs.ethers.io/v5/api/utils/address/#utils--contract-addresses)) pre-compute the contract address on the client-side, based
on sender address and nonce. In the Hedera ecosystem, it's not like that, where it's just the next available id.

##### The JSON RPC relay serves as an interface to the Hedera network for ethereum developer tools that utilize the implemented JSON RPC APIs. The following development tools have been tested and the extent of their coverage is noted below.
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: this line should be above the Supported tools as the first statement

Signed-off-by: nikolay <n.atanasow94@gmail.com>
@sonarcloud
Copy link

sonarcloud bot commented Aug 10, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@natanasow natanasow merged commit 6316fe7 into main Aug 10, 2022
@natanasow natanasow deleted the 380-comparison-analysis-table-for-supported-tools branch August 10, 2022 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request limechain P2
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Comparison analysis table for supported tools
5 participants