-
Notifications
You must be signed in to change notification settings - Fork 42
Adding form_submission prerendering explainer #418
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
Conversation
tunetheweb
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.
Looks mostly good to me, with some nits.
|
This generally looks good to me. I've tweaked one of @tunetheweb's code suggestions above just a tiny bit, but I think we can land this explainer once the rest are resolved. I'd clarify in the title of this PR that this is just an explainer addition though, and that we're not actually adding the feature to the spec yet (which is what the current title kind of suggests). |
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
tunetheweb
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.
Some more nits but overall LGTM so going ahead and approving.
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
|
Thanks! I think this is good to merge. Will let @domfarolino have one more chance to review and then merge away. |
No description provided.