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

Enrich generateBothCallAndSend feature in TruffleJsonFunctionWrapperGenerator #1986

Merged
merged 5 commits into from
Mar 27, 2024

Conversation

tonykwok1992
Copy link
Contributor

What does this PR do?

  • Enrich generateBothCallAndSend feature in TruffleJsonFunctionWrapperGenerator
  • Refactor arg parsing logic using picoli rather than hand crafted parsing logic
  • Make CommandLine ExceptionHandler re-throw rather than system exit so that test cases can catch failure case caused by exception

Where should the reviewer start?

All files

Why is it needed?

To be able to use web3j generate truffle with flag --generateBoth / -B to generator both send and call in wrapper generation using truffle option

Copy link

codecov bot commented Mar 13, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 69.22%. Comparing base (c0679c0) to head (0dcfeee).

Files Patch % Lines
...j/codegen/TruffleJsonFunctionWrapperGenerator.java 66.66% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1986      +/-   ##
============================================
- Coverage     69.23%   69.22%   -0.02%     
+ Complexity     3125     3120       -5     
============================================
  Files           493      493              
  Lines         13156    13151       -5     
  Branches       1696     1694       -2     
============================================
- Hits           9109     9104       -5     
- Misses         3557     3566       +9     
+ Partials        490      481       -9     

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

Copy link
Contributor

@NickSneo NickSneo left a comment

Choose a reason for hiding this comment

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

LGTM

@NickSneo
Copy link
Contributor

@tonykwok1992 Please rebase to master and do spotlessApply

@tonykwok1992 tonykwok1992 force-pushed the truffle-generate-both branch from d73db24 to 3fddbda Compare March 14, 2024 13:27
@tonykwok1992
Copy link
Contributor Author

rebased

@tonykwok1992 tonykwok1992 force-pushed the truffle-generate-both branch from 3fddbda to 594bd93 Compare March 14, 2024 14:10
@tonykwok1992
Copy link
Contributor Author

rebased again

@@ -40,6 +42,8 @@

import static org.web3j.codegen.Console.exitError;
import static org.web3j.utils.Collection.tail;
import static picocli.CommandLine.*;
Copy link
Contributor

Choose a reason for hiding this comment

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

Plz remove this wildcard import

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

private String destinationDirLocation;

@Option(
names = {"-B", "--generateBoth"},
Copy link
Contributor

Choose a reason for hiding this comment

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

rename this to generateCallAndSend with "-cs"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it will be better to be consistent with SolidityFunctionWrapperGenerator which use "-B", "--generateBoth" ?

https://github.com/web3j/web3j/blob/master/codegen/src/main/java/org/web3j/codegen/SolidityFunctionWrapperGenerator.java#L263

@tonykwok1992 tonykwok1992 force-pushed the truffle-generate-both branch from 594bd93 to 0dcfeee Compare March 22, 2024 16:19
@tonykwok1992
Copy link
Contributor Author

rebased

@NickSneo NickSneo merged commit 8e9759d into hyperledger-web3j:master Mar 27, 2024
4 of 6 checks passed
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.

3 participants