Skip to content

Conversation

@joehan
Copy link
Contributor

@joehan joehan commented May 9, 2022

Description

Reviewed the staged HTTPS docs, and found +fixed the following issues:
Formatting Issues:

  • FunctionsErrorCode includes bullet points that were being rendered poorly. Added newlines to fix the formatting
    Missing descriptions:
    onCall
    onRequest
    httpError.toJson - Added a brief description
    https.HttpsOptions.cors - Added an explanation of what this option does with various types - please double check this to make sure I'm correct
    request.rawBody - added a brief description

@joehan joehan requested review from TheIronDev and inlined May 9, 2022 18:28
@joehan joehan requested a review from inlined May 9, 2022 20:15

/**
* Handles HTTPS requests.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Please document req/res

@joehan joehan requested a review from egilmorez May 9, 2022 22:21
Copy link
Contributor

@egilmorez egilmorez left a comment

Choose a reason for hiding this comment

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

Some thoughts Joe, thanks!

Kudos for staging and eyeballing the lists (I'm assuming you did that).

@joehan
Copy link
Contributor Author

joehan commented May 9, 2022

@egilmorez Addressed feedback. I haven't staged the list changes yet - I just manually reviewed the MD files and verified that the dcogen added linebreaks where expected. I'll take a crack at that now - however, my desktop is not in working condition RN so I may run into some issues.

Copy link
Contributor

@egilmorez egilmorez left a comment

Choose a reason for hiding this comment

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

LG, thanks!

@joehan joehan merged commit 4825fb4 into master May 10, 2022
@joehan joehan deleted the jh-https-docs branch May 10, 2022 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants