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

Bring Logging samples up to standard. #183

Merged
merged 1 commit into from
Aug 21, 2016
Merged

Bring Logging samples up to standard. #183

merged 1 commit into from
Aug 21, 2016

Conversation

jmdobry
Copy link
Member

@jmdobry jmdobry commented Aug 19, 2016

Fixes #175

  • - Sinks
  • - Logs

@jmdobry
Copy link
Member Author

jmdobry commented Aug 19, 2016

@ace-n Ready for review, when you have a moment.

@codecov-io
Copy link

codecov-io commented Aug 20, 2016

Current coverage is 87.42% (diff: 97.87%)

Merging #183 into master will increase coverage by 1.10%

@@             master       #183   diff @@
==========================================
  Files            58         57     -1   
  Lines          2383       2409    +26   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           2057       2106    +49   
+ Misses          326        303    -23   
  Partials          0          0          

Powered by Codecov. Last update 47baf19...b5f6fde

config.filter = options.filter;
}
if (options.limit) {
config.pageSize = options.limit;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we rename the options values to the names config uses? That way we can just pass options in as config.

Copy link
Member Author

@jmdobry jmdobry Aug 21, 2016

Choose a reason for hiding this comment

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

Tried that. gcloud-node throws an error if the options contains keys that aren't official options.


'use strict';

var gcloud = require('google-cloud');
Copy link
Contributor

@ace-n ace-n Aug 21, 2016

Choose a reason for hiding this comment

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

😢 (is there any way to not require the whole thing?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, but would have to do this instead:

var Logging = require('@google-cloud/logging');
var Storage = require('@google-cloud/storage');
var BigQuery = require('@google-cloud/bigquery');
var PubSub = require('@google-cloud/pubsub');

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, makes sense.

@ace-n
Copy link
Contributor

ace-n commented Aug 21, 2016

LGTM, other than my comments.

@jmdobry
Copy link
Member Author

jmdobry commented Aug 21, 2016

Cool. Just pushed again, will merge when green.

@jmdobry
Copy link
Member Author

jmdobry commented Aug 21, 2016

Thanks!

NimJay pushed a commit that referenced this pull request Nov 18, 2022
NimJay pushed a commit that referenced this pull request Nov 18, 2022
jsimonweb pushed a commit to jsimonweb/nodejs-docs-samples that referenced this pull request Dec 12, 2022
kweinmeister pushed a commit that referenced this pull request Jan 11, 2023
🤖 I have created a release *beep* *boop*
---


## [2.0.0](googleapis/nodejs-retail@v1.8.1...v2.0.0) (2022-06-20)


### ⚠ BREAKING CHANGES

* update library to use Node 12 (#181)

### Features

* allow users to disable spell check in search requests ([#183](googleapis/nodejs-retail#183)) ([05005ea](googleapis/nodejs-retail@05005ea))


### Bug Fixes

* **deps:** update dependency @google-cloud/bigquery to v6 ([#186](googleapis/nodejs-retail#186)) ([fc07923](googleapis/nodejs-retail@fc07923))


### Build System

* update library to use Node 12 ([#181](googleapis/nodejs-retail#181)) ([809853f](googleapis/nodejs-retail@809853f))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
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.

3 participants