Skip to content

feat: Command improvements#968

Draft
thibault-martinez wants to merge 10 commits intomove-core-sdkfrom
replace-Command
Draft

feat: Command improvements#968
thibault-martinez wants to merge 10 commits intomove-core-sdkfrom
replace-Command

Conversation

@thibault-martinez
Copy link
Member

No description provided.

/// argument-nested-result = %x03 u16 u16
/// ```
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
#[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)]
Copy link
Contributor

Choose a reason for hiding this comment

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

style: not sure why Clone, Copy come last in this particular case?

Comment on lines +1220 to +1232
pub fn non_system_packages_to_be_published(&self) -> Option<&Vec<Vec<u8>>> {
match self {
Command::Publish(cmd) => Some(&cmd.modules),
Command::Upgrade(cmd) => Some(&cmd.modules),
Command::MoveCall(_)
| Command::TransferObjects(_)
| Command::SplitCoins(_)
| Command::MergeCoins(_)
| Command::MakeMoveVector(_) => None,
}
}

pub fn is_input_arg_used(&self, input_arg: u16) -> bool {
Copy link
Member

Choose a reason for hiding this comment

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

Add some comments like
/// If this is a Publish or Upgrade command, return the modules to
/// publish. Returns None for all other command types.

/// Returns `true` if the command uses an input argument at the given index.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is another one of those methods I think should be defined locally, unless this is somehow used everywhere

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.

4 participants