-
Notifications
You must be signed in to change notification settings - Fork 442
feat(boards2): add gno.land/p/gnoland/boards package
#4821
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
base: master
Are you sure you want to change the base?
Conversation
🛠 PR Checks SummaryAll Automated Checks passed. ✅ Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🟢 Maintainers must be able to edit this pull request (more info) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This property is not being used in boards and it doesn't make a lot of sense for it to be public. It can be reintroduced if needed.
gno.land/p/gnoland/boards2 packagegno.land/p/gnoland/boards package
File tests were required to define permissions to be able to created boards, because permissions implementations have crossing methods which can't be defined within a package unit test.
gno.land/p/gnoland/boards packagegno.land/p/gnoland/boards package
Removed because this type of functionality must be implemented by the realm to keep package smaller and less opinionated.
Removed because this type of functionality must be implemented by the realm to keep package smaller and less opinionated. Also, implementing save repost would require a reference to where boards are stored or a reference to the original board instead of OriginalBoardID field, to be able to get the original thread which would be required to implement save for a repost.
| // NewReply creates a new reply to a thread or another reply. | ||
| func NewReply(parent *Post, creator address, body string) (*Post, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something to consider here could be to maybe define NewReply(), NewThread() and NewRepost() in the Boards2 realm, because they have logic which could be considered realm related if you think of boards package as a generic minimal package.
It might be helpful to hear second opinions.
jefft0
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR has tests which pass. This is based on existing realm. No conflicts with master. CI checks pass. Ready for core devs to review.
Setters allows modifying post or board properties when instances are saved within another realm. This is required in those cases because instances are readonly tainted when getted from other realm, for example data realms. Metadata adds optional flexibility to devs.
The
gno.land/p/gnoland/boardspackage is intended to be a part of Boards2 realm.It would allow making the realm smaller by defining common types within the package.
A follow up PR would refactor Boards2 realm to use this package.
Types are kept minimal so specific logic related to saving or deleting is implemented by the realm.
Package initially had support for deleting and saving but it was removed in favor of a more light weight and less opinionated boards package.
The original idea of having a generic
Posttype that can represent a thread, repost or reply is kept within the package.Repost, flagging and replies support are optional features for
Postwhich could be initialized when needed by the realm that imports boards package. A storage interface and a default implementations are available for each one of these.