-
Notifications
You must be signed in to change notification settings - Fork 531
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
Add Q&A (Installation) discussion form #4877
Add Q&A (Installation) discussion form #4877
Conversation
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.
As @gp201 suggest me in last welfare meeting to do something similar for oppia-android.
@seanlip @gp201 @BenHenning PTAL, I make form similar as per oppia-web.
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.
Thanks @MohitGupta121! This is interesting. Left a few comments after taking a first pass, PTAL.
@MohitGupta121 I don't think you need a review from me on this. Please check with @gp201 on any dev-workflow-specific issues you run into, but otherwise, once @BenHenning approves this I think you can merge it. Thanks! |
@seanlip okay thanks, Waiting for @BenHenning and @gp201 review. |
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
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.
Thanks @MohitGupta121. I think this looks really good! Excited to see how it works in practice.
Though it looks like you have a CI failure @MohitGupta121. Please fix & assign back. |
Yes @BenHenning I see the checks are failing saying due to CODEOWNER files. So how I can passed that test? I think we need to need CODEOWNER for this file mentioned in the CI Check Failure What do you suggest Ben is that correct, who is the CODEOWNER? |
@BenHenning PTAL, all checks are passed now. |
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.
Thanks @MohitGupta121. This LGTM!
Explanation
Add Q&A (Installation) discussion form
Essential Checklist
Proof that changes are correct
If your PR includes UI-related changes, then: