Skip to content

Conversation

NeverHappened
Copy link
Contributor

@NeverHappened NeverHappened commented Aug 27, 2024

@NeverHappened NeverHappened force-pushed the feat/chain-manager-cron-stargate branch from 94286af to a6a80d4 Compare September 3, 2024 14:44
@pr0n00gler
Copy link
Collaborator

Copy link
Contributor

@swelf19 swelf19 left a comment

Choose a reason for hiding this comment

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

LGTM.

} else if typed_proposal.type_field.as_str() == MSG_TYPE_REMOVE_SCHEDULE {
if strategy.has_cron_remove_schedule_permission() {
Ok(())
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM. But i think it's possible to save a couple of lines of code. If you remove all 3

} else {
            Err(ContractError::Unauthorized {})
        }

and just return the error Err(ContractError::Unauthorized {}) at the very end if non of if matches

@pr0n00gler pr0n00gler merged commit ccc6b9a into main Sep 17, 2024
13 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