feat: Add group module proto definitions and basic types #9631

blushi · 2021-07-05T11:21:39Z

Description

Following up on #9089, this PR is the first step towards the migration of x/group into the SDK. It introduces the group module proto definitions (types, tx, query) and other types.
The rest of the code (module, server, client, genesis...) is dependent on various other discussions (#9238, #9182, #9237, #7773) and will be added in follow-up PRs incrementally.

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

confirmed the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage
manually tested (if applicable)

codecov · 2021-07-05T12:02:57Z

Codecov Report

Merging #9631 (a2998c7) into master (d13d488) will decrease coverage by 0.02%.
The diff coverage is 4.16%.

@@            Coverage Diff             @@
##           master    #9631      +/-   ##
==========================================
- Coverage   63.41%   63.39%   -0.03%     
==========================================
  Files         571      572       +1     
  Lines       37542    37551       +9     
==========================================
- Hits        23808    23806       -2     
- Misses      11876    11887      +11     
  Partials     1858     1858

Impacted Files	Coverage Δ
types/tx/msgs.go	`0.00% <0.00%> (ø)`
types/tx/types.go	`6.31% <0.00%> (+0.37%)`	⬆️
x/auth/tx/builder.go	`73.01% <33.33%> (-0.43%)`	⬇️

aaronc

How about let's add a separate go.mod?

aaronc · 2021-07-06T12:38:11Z

proto/cosmos/group/v1beta1/query.proto

@@ -0,0 +1,244 @@
+syntax = "proto3";
+
+package cosmos.group.v1beta1;


Let's not release any more beta protos to production. We can keep this as is for now as long as we make sure to change to v1 after beta QA.

sounds good

blushi · 2021-07-07T07:38:00Z

How about let's add a separate go.mod?

We had this discussion with the orm package too and decided during some internal regen call to have it as a separate go module only when we migrate everything else (eg sdk modules) https://www.notion.so/regennetwork/ec16c9e6890f4241990e6575ff9b5cdc?v=bdd8303495c14680a6c92c220ee13458&p=5cb9a2cb6969475ba0317ec6d0fa2aa2
So I'm wondering if that should be the same for x/group or not? I don't have a strong opinion on that.

x/group/errors.go

tac0turtle

utACK. Love the flatter module layout!

tac0turtle · 2021-07-08T08:46:12Z

So I'm wondering if that should be the same for x/group or not? I don't have a strong opinion on that.

lets keep it under one go.mod for now. I dont think there has been a decision on modules as separate go.mods in the sdk.

aaronc · 2021-07-08T19:45:34Z

So I'm wondering if that should be the same for x/group or not? I don't have a strong opinion on that.

lets keep it under one go.mod for now. I dont think there has been a decision on modules as separate go.mods in the sdk.

Let's discuss at the next architecture call. My thinking is that separate go.mod's will enable a quicker initial release cycle for new modules like x/group and x/nft. Do you have any objections @marbar3778 ?

aaronc · 2021-07-08T19:47:28Z

I would like to add a MsgLeaveGroup via which one member can unilaterally leave the group. Opening a separate issue for this as I added automerge.

ryanchristo · 2021-07-08T22:07:30Z

proto/cosmos/group/v1beta1/types.proto

+        // TODO: do we want to support a withdrawn operation?
+        // A proposal can be deleted before the voting start time by the owner. When this happens the final status
+        // is Withdrawn.
+        //        STATUS_WITHDRAWN = 4 [(gogoproto.enumvalue_customname) = "Withdrawn"];


Is this still an open question? Should we be tracking this with an issue?

good catch @ryanchristo! I have created an issue to track that: #9660

tac0turtle · 2021-07-09T07:59:46Z

Let's discuss at the next architecture call. My thinking is that separate go.mod's will enable a quicker initial release cycle for new modules like x/group and x/nft. Do you have any objections @marbar3778 ?

I like the idea of getting it out the door sooner! Lets give it a try

Add group proto definitions and basic types

3464d4e

github-actions bot added C:x/auth C:x/gov labels Jul 5, 2021

blushi changed the title ~~ADR 042 - Add group module proto definitions and basic types~~ feat: Add group module proto definitions and basic types Jul 5, 2021

blushi added 2 commits July 5, 2021 13:36

Add files

ab360a4

Merge branch 'master' into marie/7633-group-types

72d1adb

github-actions bot removed the C:x/gov label Jul 5, 2021

Lint

4af36ff

Merge branch 'master' into marie/7633-group-types

dd432c0

blushi marked this pull request as ready for review July 5, 2021 14:52

blushi requested review from aaronc and alexanderbez as code owners July 5, 2021 14:52

blushi requested review from amaury1093, robert-zaremba, aleem1314, anilcse, atheeshp, clevinson, cmwaters, cyberbono3, fedekunze, likhita-809, tac0turtle, ruhatch and ryanchristo July 5, 2021 14:53

aaronc reviewed Jul 6, 2021

View reviewed changes

technicallyty assigned ryanchristo and technicallyty Jul 6, 2021

tac0turtle reviewed Jul 8, 2021

View reviewed changes

x/group/errors.go Outdated Show resolved Hide resolved

tac0turtle approved these changes Jul 8, 2021

View reviewed changes

blushi added 2 commits July 8, 2021 12:45

Update errors codes

c1b90e2

Merge branch 'master' into marie/7633-group-types

80756a5

aaronc approved these changes Jul 8, 2021

View reviewed changes

Merge branch 'master' into marie/7633-group-types

a2998c7

aaronc added the A:automerge Automatically merge PR once all prerequisites pass. label Jul 8, 2021

ryanchristo reviewed Jul 8, 2021

View reviewed changes

mergify bot merged commit 402148a into master Jul 9, 2021

mergify bot deleted the marie/7633-group-types branch July 9, 2021 07:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add group module proto definitions and basic types #9631

feat: Add group module proto definitions and basic types #9631

blushi commented Jul 5, 2021 •

edited

Loading

codecov bot commented Jul 5, 2021 •

edited

Loading

aaronc left a comment

aaronc Jul 6, 2021

blushi Jul 7, 2021

blushi commented Jul 7, 2021 •

edited

Loading

tac0turtle left a comment

tac0turtle commented Jul 8, 2021

aaronc commented Jul 8, 2021

aaronc commented Jul 8, 2021 •

edited

Loading

ryanchristo Jul 8, 2021

blushi Jul 9, 2021

tac0turtle commented Jul 9, 2021

		@@ -0,0 +1,244 @@
		syntax = "proto3";

		package cosmos.group.v1beta1;

feat: Add group module proto definitions and basic types #9631

feat: Add group module proto definitions and basic types #9631

Conversation

blushi commented Jul 5, 2021 • edited Loading

Description

Author Checklist

Reviewers Checklist

codecov bot commented Jul 5, 2021 • edited Loading

Codecov Report

aaronc left a comment

Choose a reason for hiding this comment

aaronc Jul 6, 2021

Choose a reason for hiding this comment

blushi Jul 7, 2021

Choose a reason for hiding this comment

blushi commented Jul 7, 2021 • edited Loading

tac0turtle left a comment

Choose a reason for hiding this comment

tac0turtle commented Jul 8, 2021

aaronc commented Jul 8, 2021

aaronc commented Jul 8, 2021 • edited Loading

ryanchristo Jul 8, 2021

Choose a reason for hiding this comment

blushi Jul 9, 2021

Choose a reason for hiding this comment

tac0turtle commented Jul 9, 2021

blushi commented Jul 5, 2021 •

edited

Loading

codecov bot commented Jul 5, 2021 •

edited

Loading

blushi commented Jul 7, 2021 •

edited

Loading

aaronc commented Jul 8, 2021 •

edited

Loading