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.

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): hall of fame #2842

Open
wants to merge 74 commits into
base: master
Choose a base branch
from
Open

feat(examples): hall of fame #2842

wants to merge 74 commits into from

Conversation

leohhhn
Copy link
Contributor

@leohhhn leohhhn commented Sep 24, 2024

Description

Depends on #2584 for avlpager

Introduces the r/demo/hof realm.

The Hall of Fame is an exhibition that holds items. Users can add their realms to the Hall of Fame by importing the Hall of Fame realm and calling hof.Register() from their init function.

The realm is moderated and the registrations be paused at will.

Screenshot 2024-10-07 at 20 09 43

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.

@leohhhn leohhhn self-assigned this Sep 24, 2024
@github-actions github-actions bot added the 🧾 package/realm Tag used for new Realms or Packages. label Sep 24, 2024
@leohhhn leohhhn linked an issue Sep 24, 2024 that may be closed by this pull request
Copy link

codecov bot commented Sep 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.11%. Comparing base (5444859) to head (1cc0a9e).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2842   +/-   ##
=======================================
  Coverage   61.10%   61.11%           
=======================================
  Files         564      564           
  Lines       75355    75351    -4     
=======================================
+ Hits        46045    46050    +5     
+ Misses      25946    25937    -9     
  Partials     3364     3364           
Flag Coverage Δ
contribs/gnodev 60.65% <ø> (-0.82%) ⬇️
contribs/gnofaucet 15.31% <ø> (ø)
gnovm 66.17% <ø> (ø)
misc/genstd 80.54% <ø> (ø)
misc/logos 19.88% <ø> (ø)
tm2 62.03% <ø> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@leohhhn leohhhn marked this pull request as ready for review October 7, 2024 18:05
@leohhhn leohhhn requested review from a team as code owners October 7, 2024 18:05
@leohhhn leohhhn requested review from aeddi and piux2 and removed request for a team October 7, 2024 18:05
@leohhhn leohhhn changed the title feat(examples): gno.land hall of fame feat(examples): hall of fame Oct 7, 2024
examples/gno.land/r/gnoland/hof/rendering.gno Outdated Show resolved Hide resolved
)

const (
likesBar = "#### [%d 👍](/r/demo/hof?help&__func=Upvote&pkgpath=%s) - [%d 👎](/r/demo/hof?help&__func=Downvote&pkgPath=%s)\n\n"
Copy link
Member

Choose a reason for hiding this comment

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

for this, you'll be able to use #2887

Copy link
Member

Choose a reason for hiding this comment

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

Can you consider reviewing #2887 so we can use it in this PR?


// Must not yet exist and must be called from code
if submission.IsUser() {
panic(ErrNonCodeCall.Error())
Copy link
Member

Choose a reason for hiding this comment

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

I think we should not panic about registration but instead return an error.

If someone has a conditional registration that can occur 0 or N times, it will brick their contract.

Additionally, when we add "upgradeable contracts," the init() function can be called multiple times.

I suggest returning an error but ignoring it from the caller by default, as we don't need to address it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#2842 (comment)

What about this? :D

Copy link
Member

Choose a reason for hiding this comment

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

I'm sorry if I initially said something wrong; I don't recall the context.

However, Register should not panic. Let's avoid returning anything and don't panic.

The goal is to ensure you're registered at launch without needing complex logic to check if you were already registered. It's similar to a "Set" or "Upsert" in other systems (can override with the same value), or sending a "wakeup" UDP packet where it doesn't matter if the system is already up and running.

Copy link
Contributor Author

@leohhhn leohhhn Oct 15, 2024

Choose a reason for hiding this comment

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

@leohhhn leohhhn requested a review from jaekwon as a code owner October 8, 2024 10:21
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: No status
Status: In Review
Development

Successfully merging this pull request may close these issues.

"home of fame", an hall of fame for homepages.
4 participants