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

Simplify function doc samples #717

Closed
wants to merge 8 commits into from
Closed

Simplify function doc samples #717

wants to merge 8 commits into from

Conversation

grant
Copy link
Contributor

@grant grant commented Aug 22, 2018

  • Shorthand === undefined
  • Use maps rather than switch statements

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 22, 2018
@@ -23,14 +23,14 @@
* @param {Object} res ExpressJS object containing the HTTP response to send.
*/
exports.helloWorld = (req, res) => {
if (req.body.message === undefined) {
if (req.body.message) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing an !?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

name = req.body.name;
break;

let name = {
Copy link
Contributor

Choose a reason for hiding this comment

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

@jmdobry @MylesBorins I like this pattern - is it sufficiently idiomatic?

Copy link
Contributor

Choose a reason for hiding this comment

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


res.status(200).send(`Hello ${name || 'World'}!`);
};
// [END functions_http_content]

// [START functions_http_method]
function handleGET (req, res) {
function handleGET(req, res) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this. This is enforced by our linter (semistandard).

@grant
Copy link
Contributor Author

grant commented Aug 23, 2018

I'll have to fix the lint errors.

@grant
Copy link
Contributor Author

grant commented Sep 11, 2018

I'm not sure exactly how to debug the failing tests, and this PR is only a stylistic change, so I'll close it.

There's something failing here:

   36:   const logs = childProcess.execSync(`functions-emulator logs read helloGCS --start-time ${startTime}`).toString();
   37:   t.true(logs.includes(`File ${filename} uploaded.`));
   38: });

  Value is not `true`:

  false

  logs.includes(`File ${filename} uploaded.`)
  => false

  `File ${filename} uploaded.`
  => 'File e176db98-2784-491d-91fd-15ee2febcc8f uploaded.'

  filename
  => 'e176db98-2784-491d-91fd-15ee2febcc8f'

@grant grant closed this Sep 11, 2018
Shabirmean pushed a commit that referenced this pull request Feb 16, 2023
See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: bencoe@google.com <bencoe@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants