-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix naming conflicts in generate code #301
Fix naming conflicts in generate code #301
Conversation
> * update macro code > * move Ownable2Step module to ownable.rs
@coderabbitai review |
Warning Rate Limit Exceeded@kpob has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 2 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe recent updates involve a significant overhaul of the ownership transfer mechanism, with a focus on a two-step transfer process. The Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (8)
- modules/src/access/ownable.rs (2 hunks)
- modules/src/lib.rs (1 hunks)
- odra-macros/src/ast/deployer_item.rs (2 hunks)
- odra-macros/src/ast/deployer_utils.rs (3 hunks)
- odra-macros/src/ast/exec_parts.rs (1 hunks)
- odra-macros/src/ast/module_impl_item.rs (2 hunks)
- odra-macros/src/ast/test_parts.rs (4 hunks)
- odra-macros/src/ast/wasm_parts.rs (6 hunks)
Files skipped from review due to trivial changes (1)
- modules/src/lib.rs
Additional comments: 22
odra-macros/src/ast/module_impl_item.rs (2)
3-3: The addition of
ExecPartsItem
to the imports suggests that it is now being used within this file. Ensure that this new import is indeed utilized in the subsequent code to avoid unused imports.19-19: The
exec_parts_reexport
field has been removed from theModuleImplItem
struct. Confirm that this field is no longer necessary and that its removal does not affect any functionality that depends on it.odra-macros/src/ast/deployer_item.rs (2)
94-106: The function calls within the match statement have been updated to use the
__erc20_exec_parts
module prefix. This change likely resolves the naming conflicts mentioned in the PR objectives. Ensure that the__erc20_exec_parts
module is correctly defined and accessible from this context.145-149: Similar to the previous comment, the function calls within this match statement have been updated to use the
__erc20_exec_parts
module prefix. Verify that these changes are consistent throughout the codebase and that the__erc20_exec_parts
module is correctly defined and accessible.odra-macros/src/ast/deployer_utils.rs (3)
67-67: The
entrypoint_caller
function now returns asyn::Result<syn::Expr>
instead ofResult<syn::Expr, syn::Error>
. This change standardizes the error type with the rest of the codebase, assumingsyn::Error
is the only error type used.76-78: The collection of
CallerBranch
instances now includes error handling, which will propagate any errors encountered during the mapping process. This is a good practice as it ensures that errors are not silently ignored and are handled appropriately.182-193: The
FunctionCallBranch
struct now implements theTryFrom
trait, and itscall_stmt
method returns asyn::Result<syn::Stmt>
instead ofsyn::Stmt
. This change enhances error handling by allowing thecall_stmt
method to return errors, which is consistent with the rest of the codebase.odra-macros/src/ast/exec_parts.rs (1)
- 10-12: The
ExecPartsReexportItem
struct and its associated logic have been removed. Confirm that this struct is no longer required and that its removal does not negatively impact any areas of the code that may have depended on the reexport functionality it provided.odra-macros/src/ast/test_parts.rs (4)
14-14: The
attr
field in theTestPartsReexportItem
struct has been renamed tonot_wasm_attr
. This renaming makes the purpose of the attribute clearer, indicating that it is related to non-WASM targets.24-24: The
not_wasm_attr
field is being used to apply an attribute to the reexport statement. Ensure that this attribute is correctly applied in all relevant contexts and that the renaming does not affect any conditional compilation logic.180-192: Function calls within the
Erc20Deployer
struct'sinit
method have been updated to use the__erc20_exec_parts
module prefix. This change is consistent with the PR objectives to resolve naming conflicts. Verify that the__erc20_exec_parts
module is correctly defined and accessible from this context.306-310: Similar to the previous comment, function calls within the
Erc20Deployer
struct'sinit
method have been updated to use the__erc20_exec_parts
module prefix. Again, verify that the__erc20_exec_parts
module is correctly defined and accessible.modules/src/access/ownable.rs (2)
1-4: The import statements have been updated to include
CallerNotTheNewOwner
andOwnershipTransferStarted
, which are new or renamed entities. Additionally, theOwnable
module now emits theOwnershipTransferStarted
event. Ensure that these changes are reflected throughout the codebase and that the new event is handled correctly where necessary.79-152: The
Ownable2Step
struct has been added, which includes aModuleWrapper<Ownable>
and apending_owner
variable for two-step ownership transfer. New functions such astransfer_ownership
andaccept_ownership
have been introduced, along with theOwnershipTransferStarted
event. Verify that the two-step ownership transfer logic is correctly implemented and that the new functions and events are integrated properly with the rest of the system.odra-macros/src/ast/wasm_parts.rs (8)
51-57: The addition of
.map(|f| (module, f))
in theentry_points
collection process is a logical change that aligns with the newTryFrom
implementation forNoMangleFnItem
which now requires a tuple ofModuleImplIR
andFnIR
. This ensures that the necessary context is passed to createNoMangleFnItem
instances.160-175: The changes to the
try_from
method ofNoMangleFnItem
correctly use theexec_parts_ident
to prefix the function calls, ensuring that the functions are called from the correct module. This is a necessary change to resolve the naming conflicts mentioned in the PR objectives.347-347: The update to prefix the
execute_init
function call with__erc20_exec_parts::
is consistent with the changes made to resolve naming conflicts. This ensures that the function is called from the correct module.352-352: The update to prefix the
execute_total_supply
function call with__erc20_exec_parts::
is consistent with the changes made to resolve naming conflicts. This ensures that the function is called from the correct module.362-362: The update to prefix the
execute_pay_to_mint
function call with__erc20_exec_parts::
is consistent with the changes made to resolve naming conflicts. This ensures that the function is called from the correct module.367-367: The update to prefix the
execute_approve
function call with__erc20_exec_parts::
is consistent with the changes made to resolve naming conflicts. This ensures that the function is called from the correct module.425-425: The update to prefix the
execute_total_supply
function call with__erc20_exec_parts::
in the test trait implementation is consistent with the changes made to resolve naming conflicts. This ensures that the function is called from the correct module.435-435: The update to prefix the
execute_pay_to_mint
function call with__erc20_exec_parts::
in the test trait implementation is consistent with the changes made to resolve naming conflicts. This ensures that the function is called from the correct module.
Before, keeping Ownable and Ownable2Step in the module did not work due to naming conflicts. Some entrypoints have the same name, and reexporting
execute_
functions causes an error.Keeping them inside separate modules resolves the issue. The only downside is using fully qualified paths when calling functions.
Summary by CodeRabbit
New Features
Refactor
Documentation
ownable_2step
module from the public documentation.Style