-
Notifications
You must be signed in to change notification settings - Fork 399
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 {p,r}/agherasie/forms #3524
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: Leon Hudak <33522493+leohhhn@users.noreply.github.com>
Co-authored-by: Leon Hudak <33522493+leohhhn@users.noreply.github.com>
Co-authored-by: Guilhem Fanton <8671905+gfanton@users.noreply.github.com>
Co-authored-by: Guilhem Fanton <8671905+gfanton@users.noreply.github.com>
Hi @notJoon thanks so much for your suggestions ! |
@notJoon let's keep the "triage pending" while you folks are still reviewing it ;) |
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.
LGTM 👍
removing the review/triage-pending
flag because the review is complete.
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.
Just check the last few comments, otherwise looks good.
You should consider moving this to your own namespace, and also calling r/leon/hof.Register()
inside the realm, in init()
so that it shows up on the Hall of Fame :)
@agherasie did you have a chance to check the latest comments? |
Hi, sorry for the delay ! I tried to quickly address the comments in my latest commits, however I am running into a problem with the tests. Not sure what it's due to yet ! |
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.
code quality could be improved, but I'd avoid this PR going stale again so merging seeing other approvals.
The CI is failing because of #3240, I think we either need to remove the hof registration or fix that issue... |
Ah, I see the PR also needs updating to the latest |
Apologies again for the delay, I've been very busy this month as I've been traveling a lot !
@thehowl feel free to add comments where you think I could improve the code if you have the time; as of tomorrow I will be a lot quicker to respond and make changes. The earlier requested changes on this PR have been very insightful for me since I'm pretty inexperienced with go/gno and I was able to learn a lot, so I am open to any further suggestions on how to improve the realm ! Thanks everyone for your help, I'm really excited to finally merging it soon ! |
Let's merge this after #3854, @agherasie please add back the hof registration, it should work after we merge that :) |
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
Sorry for the long hiatus which caused #2604 to close, I moved to South Korea this year for a university exchange program and let this PR collect dust for a while.
I've addressed #2604 (comment) in 7a6a032, hoping that the PR might be ready for merge now !
If not, please let me know if any further updates should be made to the code !
As part of the student contributor program, I attempted to create a new example realm that allows the creation and submission of forms on gno !
Features
CreateForm(...)
SubmitForm(...)
GetForms(...), GetFormByID(...), GetAnswer(...)
Field Types
The system supports the following field types:
{"label": "Name", "fieldType": "string", "required": true}
{"label": "Age", "fieldType": "number", "required": true}
{"label": "Is Student?", "fieldType": "boolean", "required": false}
Demo
The external repo where the initial development took place and where you can find the frontend is here.
The web app itself is hosted here
And the most recent test4 version of the contract is forms2
Screenshots :
gnoweb Render()
a form response in the web interface
creating a form in the web interface
Contributors' checklist...
BREAKING CHANGE: xxx
message was included in the description