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

Add CommunitySpendProposals in migration #7607

Merged
merged 4 commits into from
Oct 20, 2020
Merged

Add CommunitySpendProposals in migration #7607

merged 4 commits into from
Oct 20, 2020

Conversation

amaury1093
Copy link
Contributor

@amaury1093 amaury1093 commented Oct 20, 2020

Description

closes: #7601

Add CommunitySpendProposals in v036 distribution, and add correct migration paths until v040.


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/<module>/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

@amaury1093 amaury1093 marked this pull request as ready for review October 20, 2020 10:24
@amaury1093 amaury1093 requested review from zmanian and anilcse October 20, 2020 10:24
@codecov
Copy link

codecov bot commented Oct 20, 2020

Codecov Report

Merging #7607 into master will increase coverage by 0.02%.
The diff coverage is 91.66%.

@@            Coverage Diff             @@
##           master    #7607      +/-   ##
==========================================
+ Coverage   54.19%   54.21%   +0.02%     
==========================================
  Files         610      611       +1     
  Lines       38363    38461      +98     
==========================================
+ Hits        20791    20852      +61     
- Misses      15465    15500      +35     
- Partials     2107     2109       +2     

Copy link
Collaborator

@robert-zaremba robert-zaremba left a comment

Choose a reason for hiding this comment

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

Left one comment.

RouterKey = ModuleName

// ProposalTypeCommunityPoolSpend defines the type for a CommunityPoolSpendProposal
ProposalTypeCommunityPoolSpend = "CommunityPoolSpend"
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why don't we import this from x/distribution/types/keys.go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bez preferred to have migration logic self-contained and not rely on the module itself, ref: #6839 (comment).

Note that we're breaking the above rule for >=0.40, as we're now using proto types from the module for 0.40.

@amaury1093 amaury1093 added the A:automerge Automatically merge PR once all prerequisites pass. label Oct 20, 2020
@mergify mergify bot merged commit 3c43370 into master Oct 20, 2020
@mergify mergify bot deleted the am-7601-migrate branch October 20, 2020 13:19
clevinson pushed a commit that referenced this pull request Oct 20, 2020
* Add CommunitySpendProposals in migration

* Fix lint

* Avoid double registration

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
clevinson added a commit that referenced this pull request Oct 20, 2020
* Add CommunitySpendProposals in migration

* Fix lint

* Avoid double registration

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:automerge Automatically merge PR once all prerequisites pass.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CommunitySpendProposals missing from the governance migration process
4 participants