-
Notifications
You must be signed in to change notification settings - Fork 341
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
feat(examples): add memeland #1751
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1751 +/- ##
=======================================
Coverage 47.49% 47.49%
=======================================
Files 388 388
Lines 61311 61311
=======================================
Hits 29117 29117
Misses 29756 29756
Partials 2438 2438 ☔ View full report in Codecov by Sentry. |
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.
Overall looks good, please address the comments 🙏
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.
I know this is a PoC, but took the liberty of giving some feedback related to the data structure and ways to paginate:
- AFAIK there is no need to remove posts, I would suggest changing the
avl.Tree
to a slice of Posts. This gives several advantages, like:- No need for casting when retrieving from the slice instead of the
avl.Tree
. - Sequential index already there, it will be the slice index instead of a string.
- No rebalancing is needed.
- Easier pagination mapping directly the elements per page and the page number to the slice indexes.
- No need for casting when retrieving from the slice instead of the
- I would change
GetPosts()
method toGetPosts(page,pagesize int)
. Too risky. - I would limit the pageSize on GetPostsInRange.
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.
🚀 🚀 🚀
Description
This PR adds the
p/demo/memeland
&r/demo/memeland
package & realm to the examples folder. Adding this will be the way we "Deploy" the Memeland realm to the Portal Loop.The UI and the most up to date code can be found here. Waiting on full release and will copy code over from the other repo.
The only thing currently missing are public realm functions tested. The package is fully tested.
Contributors' checklist...
BREAKING CHANGE: xxx
message was included in the description