Skip to content

update x/gov to match module spec#4665

Merged
alexanderbez merged 42 commits intomasterfrom
gov-internal
Aug 8, 2019
Merged

update x/gov to match module spec#4665
alexanderbez merged 42 commits intomasterfrom
gov-internal

Conversation

@fedekunze
Copy link
Copy Markdown
Contributor

  • 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.

  • Wrote tests

  • Updated relevant documentation (docs/)

  • Added a relevant changelog entry: clog add [section] [stanza] [message]

  • rereviewed Files changed in the github PR explorer


For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 23, 2019

Codecov Report

Merging #4665 into master will decrease coverage by 0.25%.
The diff coverage is 60.8%.

@@            Coverage Diff             @@
##           master    #4665      +/-   ##
==========================================
- Coverage    54.3%   54.04%   -0.26%     
==========================================
  Files         271      269       -2     
  Lines       17344    17114     -230     
==========================================
- Hits         9418     9249     -169     
+ Misses       7228     7179      -49     
+ Partials      698      686      -12

@fedekunze fedekunze requested a review from rigelrozanski as a code owner July 25, 2019 10:51
@fedekunze fedekunze requested a review from colin-axner July 25, 2019 10:51
@fedekunze
Copy link
Copy Markdown
Contributor Author

To introduce the internal/ pkg, we'd have to move to exported/ the Content and Handler interfaces as well as NewMsgSubmitProposal, codec's RegisterProposalType(Codec), ValidateAbstract, etc.

Do we want to do that or should we leave it as is? @rigelrozanski @alexanderbez

Copy link
Copy Markdown
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

Blocking until review of proposal registration process

@fedekunze fedekunze added S:blocked Status: Blocked and removed S:blocked Status: Blocked labels Aug 5, 2019
@fedekunze fedekunze requested a review from alexanderbez August 6, 2019 21:15
fedekunze and others added 6 commits August 7, 2019 13:12
Co-Authored-By: colin axner <colinaxner@berkeley.edu>
Co-Authored-By: colin axner <colinaxner@berkeley.edu>
Co-Authored-By: colin axner <colinaxner@berkeley.edu>
Co-Authored-By: colin axner <colinaxner@berkeley.edu>
Co-Authored-By: colin axner <colinaxner@berkeley.edu>
Copy link
Copy Markdown
Contributor

@rigelrozanski rigelrozanski left a comment

Choose a reason for hiding this comment

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

Dopeness - much needed cleanup - left a few comments worth addressing

return 0, types.ErrInvalidGenesis(keeper.codespace, "initial proposal ID hasn't been set")
}
keeper.cdc.MustUnmarshalBinaryLengthPrefixed(bz, &proposalID)
proposalID = binary.LittleEndian.Uint64(bz)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be consolidated to a key function per my previous comments too

@alexanderbez alexanderbez added the T: State Machine Breaking State machine breaking changes (impacts consensus). label Aug 8, 2019
Copy link
Copy Markdown
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

ACK

@alexanderbez alexanderbez merged commit e4c8bd7 into master Aug 8, 2019
@alexanderbez alexanderbez deleted the gov-internal branch August 8, 2019 19:51
larry0x pushed a commit to larry0x/cosmos-sdk that referenced this pull request May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C:x/gov T: State Machine Breaking State machine breaking changes (impacts consensus). Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants