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.

Sign up for GitHub

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

Jump to bottom

feat: Added missing functionality to govDAO #2493

Merged
merged 13 commits into from
Jul 8, 2024
Merged

feat: Added missing functionality to govDAO #2493

merged 13 commits into from
Jul 8, 2024

Conversation

ajnavarro
Copy link
Contributor

@ajnavarro ajnavarro commented Jul 3, 2024 •
edited
Loading

  • Add a Voter interface to be able to implement different voting systems in the future
  • Add members and an executor to add them
  • Improve Render output a bit.
  • Added tests
  • Tidy up and remove unnecessary code
Contributors' checklist...
  • Added new tests, or not needed, or not feasible
  • Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
  • Updated the official documentation or not needed
  • No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
  • Added references to related issues and PRs
  • Provided any useful hints for running manual tests
  • Added new benchmarks to generated graphs, if any. More info here.

@ajnavarro ajnavarro changed the title Added missing functionality to govDAO feat: Added missing functionality to govDAO Jul 3, 2024
@github-actions github-actions bot added the 🧾 package/realm Tag used for new Realms or Packages. label Jul 3, 2024
@ajnavarro ajnavarro force-pushed the dev/ajnavarro/feature/govDAO-improvements branch 3 times, most recently from bfbf7ff to 6395ea0 Compare July 3, 2024 15:58
zivkovicmilos
zivkovicmilos approved these changes Jul 4, 2024
View reviewed changes
Copy link
Member

@zivkovicmilos zivkovicmilos left a comment

Choose a reason for hiding this comment

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

Looks good 💯

I've left a few questions and nits, otherwise it's a good direction

examples/gno.land/p/gov/proposal/proposal_test.gno Outdated Show resolved Hide resolved
examples/gno.land/r/gov/dao/dao.gno Outdated Show resolved Hide resolved
examples/gno.land/r/gov/dao/dao.gno Show resolved Hide resolved
examples/gno.land/r/gov/dao/dao.gno Outdated Show resolved Hide resolved
examples/gno.land/r/gov/dao/dao.gno Show resolved Hide resolved
examples/gno.land/r/gov/dao/voter.gno Show resolved Hide resolved
examples/gno.land/r/gov/dao/voter.gno Outdated Show resolved Hide resolved
examples/gno.land/r/gov/dao/voter.gno Outdated Show resolved Hide resolved
examples/gno.land/r/gov/dao/voter.gno Show resolved Hide resolved
examples/gno.land/r/gov/dao/voter.gno Outdated Show resolved Hide resolved
@ajnavarro ajnavarro marked this pull request as ready for review July 4, 2024 13:45
@ajnavarro ajnavarro requested review from a team as code owners July 4, 2024 13:45
@ajnavarro ajnavarro requested review from gfanton and petar-dambovaliev and removed request for a team July 4, 2024 13:45
@moul moul mentioned this pull request Jul 4, 2024
Merged
moul
moul approved these changes Jul 4, 2024
View reviewed changes
Copy link
Member

@moul moul left a comment

Choose a reason for hiding this comment

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

Except that I believe we should switch to an avl.Tree for voter records to improve efficiency and simplicity (i.e., PercentageVoter.alreadyVoted()), this PR is a very good improvement and we can merge it as is.

@ajnavarro
Added missing functionality to govDAO
bfb562f 
- Add a Voter interface to be able to implement different voting systems
  in the future
- Add members and an executor to add them
- Improve Render output a bit.
- Added tests
- Tidy up and remove unnecessary code

Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
@ajnavarro
Fix proposal import
025eaad 
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
@ajnavarro
Fix linter
0163835 
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
@ajnavarro
Add more tests and add requested changes.
221cd62 
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
@ajnavarro
Fix CI
045119b 
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
@ajnavarro
Added requested changes on panic messages.
002f8e0 
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
@ajnavarro
Requested changes
78994bf 
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
ajnavarro and others added 3 commits July 8, 2024 14:29
@ajnavarro
Requested changes.
c905b97 
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
@ajnavarro @moul
Update examples/gno.land/p/gov/proposal/proposal.gno
6bb2fb8 
Co-authored-by: Manfred Touron <94029+moul@users.noreply.github.com>
@ajnavarro
gno mod tidy
c001ba3 
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
@ajnavarro ajnavarro force-pushed the dev/ajnavarro/feature/govDAO-improvements branch from 3ac4dd4 to c001ba3 Compare July 8, 2024 12:31
@ajnavarro ajnavarro enabled auto-merge (squash) July 8, 2024 14:09
@ajnavarro ajnavarro merged commit ebf4d8f into master Jul 8, 2024
9 checks passed
@ajnavarro ajnavarro deleted the dev/ajnavarro/feature/govDAO-improvements branch July 8, 2024 14:12
@Kouteki Kouteki mentioned this pull request Jul 11, 2024
Open
gfanton pushed a commit to gfanton/gno that referenced this pull request Jul 23, 2024
@ajnavarro @moul @gfanton
feat: Added missing functionality to govDAO (gnolang#2493)
5f90151 
Co-authored-by: Manfred Touron <94029+moul@users.noreply.github.com>
@michelleellen michelleellen mentioned this pull request Jul 31, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leohhhn leohhhn leohhhn left review comments

@zivkovicmilos zivkovicmilos zivkovicmilos approved these changes

@moul moul moul approved these changes

@gfanton gfanton Awaiting requested review from gfanton gfanton was automatically assigned from gnolang/tech-staff

@petar-dambovaliev petar-dambovaliev Awaiting requested review from petar-dambovaliev petar-dambovaliev was automatically assigned from gnolang/tech-staff

Assignees

@ajnavarro ajnavarro

Labels
🧾 package/realm Tag used for new Realms or Packages.
Projects
💪 Bounties & Worx
Status: Done
😎 Manfred's Board
Status: ✅ Done
🧙‍♂️gno.land core team
Archived in project
Milestone
🏗4️⃣ test4.gno.land
Development

Successfully merging this pull request may close these issues.
4 participants
@ajnavarro @moul @zivkovicmilos @leohhhn