-
Notifications
You must be signed in to change notification settings - Fork 234
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
reviews for AElf.Kernel.SmartContract #2409
Conversation
rename to GrpcCrossChainNodePluginAElf/src/AElf.CrossChain.Communication.Grpc/GrpcNodePlugin.cs Lines 7 to 10 in c0a9236
This comment was generated by todo based on a
|
if TransactionExecutedEventData is only for Debug, TransactionExecutedEventData should also wrap in #DEBUGAElf/src/AElf.Kernel.Core/SmartContractExecution/Events/TransactionExecutedEventData.cs Lines 5 to 9 in c0a9236
This comment was generated by todo based on a
|
a service should not mange status. what's this class for?
This comment was generated by todo based on a
|
if you are a CalculateAlgorithmService, why do you need to care about fork? one class does one thingAElf/src/AElf.Kernel.SmartContract/Application/CalculateFeeStrategy/ICalculateAlgorithmService.cs Lines 10 to 13 in c0a9236
This comment was generated by todo based on a
|
we only need one strategy to calculate tx, read, write ... resources.AElf/src/AElf.Kernel.SmartContract/Application/CalculateFeeStrategy/ICalculateCostStrategy.cs Lines 15 to 18 in c0a9236
This comment was generated by todo based on a
|
it looks not OOPAElf/src/AElf.Kernel.SmartContract/Application/CalculateFeeStrategy/ICalculateWay.cs Lines 9 to 12 in c0a9236
This comment was generated by todo based on a
|
is this class still in use?This comment was generated by todo based on a
|
is still in use?AElf/src/AElf.Kernel.SmartContract/Application/DeployContractAddressForkCacheHandler.cs Lines 8 to 11 in c0a9236
This comment was generated by todo based on a
|
why in a SmartContract package, you put a Token class? if you create a door, can put a clock on the door,AElf/src/AElf.Kernel.SmartContract/Application/ExtraAvailableTokenProvider.cs Lines 3 to 7 in c0a9236
This comment was generated by todo based on a
|
if you new a class, why not pass parameters by constructor? you know the type of calWayDic[func.PieceKey], but why don't you use strong signature?AElf/src/AElf.Kernel.TransactionPool/Application/CalculateFeeService.cs Lines 165 to 168 in c0a9236
This comment was generated by todo based on a
|
a service should not mange status. what's this class for?
This comment was generated by todo based on a
|
is still in use?AElf/src/AElf.Kernel.SmartContract/Application/DeployContractAddressForkCacheHandler.cs Lines 8 to 11 in 40ce8b3
This comment was generated by todo based on a
|
why in a SmartContract package, you put a Token class? if you create a door, can put a clock on the door,AElf/src/AElf.Kernel.SmartContract/Application/ExtraAvailableTokenProvider.cs Lines 3 to 7 in 40ce8b3
This comment was generated by todo based on a
|
if you new a class, why not pass parameters by constructor? you know the type of calWayDic[func.PieceKey], but why don't you use strong signature?AElf/src/AElf.Kernel.TransactionPool/Application/CalculateFeeService.cs Lines 165 to 168 in 40ce8b3
This comment was generated by todo based on a
|
❌ Build AElf 1.0.0.6200 failed (commit c0acf98328 by @loning) |
No description provided.