Skip to content

Conversation

jeronimoalbi
Copy link
Member

Adds an initial README.md to better document the commondao package

@jeronimoalbi jeronimoalbi self-assigned this Aug 22, 2025
@github-actions github-actions bot added the 🧾 package/realm Tag used for new Realms or Packages. label Aug 22, 2025
@Gno2D2 Gno2D2 added the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Aug 22, 2025
@Gno2D2
Copy link
Collaborator

Gno2D2 commented Aug 22, 2025

🛠 PR Checks Summary

🔴 Pending initial approval by a review team member, or review from tech-staff

Manual Checks (for Reviewers):
  • IGNORE the bot requirements for this PR (force green CI check)
Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ Automated Checks (for Contributors):

🔴 Pending initial approval by a review team member, or review from tech-staff

☑️ Contributor Actions:
  1. Fix any issues flagged by automated checks.
  2. Follow the Contributor Checklist to ensure your PR is ready for review.
    • Add new tests, or document why they are unnecessary.
    • Provide clear examples/screenshots, if necessary.
    • Update documentation, if required.
    • Ensure no breaking changes, or include BREAKING CHANGE notes.
    • Link related issues/PRs, where applicable.
☑️ Reviewer Actions:
  1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.
📚 Resources:
Debug
Automated Checks
Pending initial approval by a review team member, or review from tech-staff

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 Not (🔴 Pull request author is a member of the team: tech-staff)

Then

🔴 Requirement not satisfied
└── 🔴 If
    ├── 🟢 Condition
    │   └── 🟢 Or
    │       ├── 🟢 User leohhhn already reviewed PR 4678 with state APPROVED
    │       ├── 🔴 At least 1 user(s) of the team tech-staff reviewed pull request
    │       └── 🔴 This pull request is a draft
    └── 🔴 Then
        └── 🔴 And
            ├── 🟢 Not (🔴 This label is applied to pull request: review/triage-pending)
            └── 🔴 At least 1 user(s) of the team tech-staff reviewed pull request

Manual Checks
**IGNORE** the bot requirements for this PR (force green CI check)

If

🟢 Condition met
└── 🟢 On every pull request

Can be checked by

  • Any user with comment edit permission

Copy link

codecov bot commented Aug 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@Kouteki Kouteki self-requested a review August 25, 2025 15:22
Copy link
Contributor

@leohhhn leohhhn left a comment

Choose a reason for hiding this comment

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

LGTM, leaving just one suggestion

VotingPeriod() time.Duration

// Tally counts the number of votes and verifies if proposal passes.
Tally(ReadonlyVotingRecord, MemberSet) (passes bool, _ error)
Copy link
Contributor

Choose a reason for hiding this comment

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

The ReadonlyVotingRecord type was confusing to me in general. If you could add a simple explanation to what it is and how to use it, that would be great.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sound good and makes sense to explain it 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Done 2e61310

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't explicitly explains why the voting record is readonly, but explains what a voting record is.

It's readonly to avoid proposal definitions to change voting information.

@Gno2D2 Gno2D2 removed the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Sep 2, 2025
Comment on lines 3 to 7
This package provides support to implement custom Decentralized Autonomous
Organizations (DAO).

It aims to be minimal and flexible, allowing the implementation of multiple DAO
use cases.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This package provides support to implement custom Decentralized Autonomous
Organizations (DAO).
It aims to be minimal and flexible, allowing the implementation of multiple DAO
use cases.
The CommonDAO is a general-purpose implementation of a Decentralized Autonomous Organization (DAO) on Gno.land. The CommonDAO package offers a minimal, and flexible framework for building and deploying DAOs, with customizable options to adapt to across multiple use cases.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done ba6b83a

Changed it a bit.

CommonDAO type is the main type used to define DAOs, allowing standalone DAO
creation or hierarchical tree based ones.

During creation it accepts many optional arguments some of which are handy
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
During creation it accepts many optional arguments some of which are handy
During creation. it accepts many optional arguments some of which are handy

Copy link
Member Author

Choose a reason for hiding this comment

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

Done ba6b83a

and description to uniquely identify individual DAOs; Hierarchical ones might
choose to use slugs instead of IDs, or even a mix of both.

Members can also optionally be initialized during DAO creation.
Copy link
Contributor

@michelleellen michelleellen Sep 4, 2025

Choose a reason for hiding this comment

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

The 'members can also ...' feels a bit out of place/disconnected from the first part of this content, or maybe it needs more context as to how it relates to the CommonDAO type with a comparison example, initialized before DAO creation vs during DAO creation?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed it because it didn't really added anything to the document and it was indeed somehow disconnected.

Done in 28aa1c8

are usually used to signal or measure sentiment, for example regarding a
relevant issue.

Creating a new proposal type requires to implement the following interface:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Creating a new proposal type requires to implement the following interface:
Creating a new proposal type requires implementing the following interface:

Copy link
Member Author

Choose a reason for hiding this comment

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

Done ba6b83a

Title() string

// Body returns proposal body.
// It usually contains the proposal description and other elements like
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth explaining a few examples of proposal parameters for people who might not know?

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed description to remove the concept of parameter and used a hopefully simpler description in 0e00c77

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧾 package/realm Tag used for new Realms or Packages.
Projects
Status: In Progress
Status: Triage
Development

Successfully merging this pull request may close these issues.

5 participants