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

Issue#674 #680

Closed
wants to merge 10 commits into from
Closed

Issue#674 #680

wants to merge 10 commits into from

Conversation

juggernaut09
Copy link
Contributor

@juggernaut09 juggernaut09 commented Dec 31, 2020

issue: #674

  • This is a Pull request with respect to issue: Replace migrate entry point with more general system #674. Added system functionality along with the migrate. So that both old and new models can use the migrate functionality.
  • Did not remove the _info: MessageInfo parameter for pub fn system(). Changed the SystemMsg from InitMsg like struct to HandleMsg like enum.
  • using match While assigning the MigrateMsg.verifier to config.verifier in pub fn system.
  • Created a new file called system.rs in packages/src/results/ mod in order to provide return value. StdResult<SystemResponse>.

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x//spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes
    Please review this PR @ethanfrey

@juggernaut09 juggernaut09 marked this pull request as ready for review January 6, 2021 05:02
Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution. I hope you had the chance to get familiar with the code base a little bit.

#674 is a ticket that was not very mature when you started working on it. We closed it now and created a few follow-up tickets.

Would you like to work on #690? This has a smaller scope and it is very clear what we want. Just let me know, then I assign you there.

@@ -17,7 +17,7 @@ pub struct InitMsg {
pub beneficiary: HumanAddr,
}

/// MigrateMsg allows a priviledged contract administrator to run
/// MigrateMsg allows a privileged contract administrator to run
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -18,4 +19,5 @@ pub use handle::{HandleResponse, HandleResult};
pub use init::{InitResponse, InitResult};
pub use migrate::{MigrateResponse, MigrateResult};
pub use query::{QueryResponse, QueryResult};
pub use system::{SystemResponse, SystemResult as SystemRes};
Copy link
Member

Choose a reason for hiding this comment

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

Ahh, good to know SystemResult already exists. We should keep that in mind and maybe find a better name for "system".

@juggernaut09
Copy link
Contributor Author

Thank you for your contribution. I hope you had the chance to get familiar with the code base a little bit.

#674 is a ticket that was not very mature when you started working on it. We closed it now and created a few follow-up tickets.

Would you like to work on #690? This has a smaller scope and it is very clear what we want. Just let me know, then I assign you there.

Yeah please, That would be great!

Base automatically changed from master to main January 19, 2021 22:43
@webmaster128
Copy link
Member

Thank you for your contribution. As discussed in #674 we keep the entry points separate. Closing.

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.

2 participants