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.

Sign up for GitHub

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

Jump to bottom

core: preserve all class members #845

Merged
merged 1 commit into from
Sep 8, 2015
Merged

core: preserve all class members #845

merged 1 commit into from
Sep 8, 2015

Conversation

callmehiphop
Copy link
Contributor

Closes #844

Unit tests need to be updated to accommodate this change, but I wanted to get some feedback before I attempted to tackle that.

I ran the system tests locally and only received a single failure for an after all hook (might be unrelated) -- so this implementation should work.

// @stephenplusplus

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 2, 2015
@callmehiphop callmehiphop mentioned this pull request Sep 2, 2015
Closed
@callmehiphop
Copy link
Contributor Author

Also might be worth mentioning that in order for this to work moving foward all our classes would have to follow the pattern..

function Class() {
  if (!(this instanceof Class)) {
    return new Class();
  }
}

@stephenplusplus
Copy link
Contributor

It looks good to me! Can we maybe wrap consistent instantiation stuff into one method? (Example to follow)

stephenplusplus
stephenplusplus reviewed Sep 2, 2015
View reviewed changes
lib/compute/index.js
if (this.config_) {
options = util.extendGlobalConfig(this.config_, options);
}

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@stephenplusplus stephenplusplus mentioned this pull request Sep 4, 2015
Closed
stephenplusplus
stephenplusplus reviewed Sep 8, 2015
View reviewed changes
lib/bigquery/index.js
@@ -96,15 +96,10 @@ var SCOPES = ['https://www.googleapis.com/auth/bigquery'];
*/
function BigQuery(options) {
if (!(this instanceof BigQuery)) {
options = util.normalizeArguments(this, options);

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

stephenplusplus added a commit that referenced this pull request Sep 8, 2015
@stephenplusplus
Merge pull request #845 from callmehiphop/config-refactor
d831777 
core: preserve all class members
@stephenplusplus stephenplusplus merged commit d831777 into googleapis:master Sep 8, 2015
@stephenplusplus
Copy link
Contributor

Woo! Bravo on this solution :)

@stephenplusplus stephenplusplus mentioned this pull request Jan 29, 2016
Closed
@sofisl sofisl mentioned this pull request Jan 17, 2023
Merged
4 tasks
sofisl pushed a commit that referenced this pull request Jan 17, 2023
@alexander-fenster
fix: do not modify options object, use defaultScopes (#845)
6f20fec 
Regenerated the library using
[gapic-generator-typescript](https://github.com/googleapis/gapic-generator-typescript)
v1.2.1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement. core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@callmehiphop @stephenplusplus @googlebot