Skip to content

Conversation

@edaniels
Copy link
Contributor

No description provided.

@edaniels edaniels requested review from adamchel and jsflax June 28, 2018 22:21
@edaniels edaniels force-pushed the STITCH-1720 branch 2 times, most recently from f42dd50 to 020480e Compare June 28, 2018 22:30
Copy link
Contributor

@adamchel adamchel left a comment

Choose a reason for hiding this comment

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

LGTM mod a few nits

import StitchUser from "../StitchUser";
import StitchUserFactoryImpl from "./StitchUserFactoryImpl";

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

[nit] Don't think we need any of these Partial things.

* Partial JSDOM window.location containing only the
* properties and method we need
*/
interface PartialLocaion {
Copy link
Contributor

Choose a reason for hiding this comment

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

[nit] assuming this same typo exists in browser, you should probably drive-by fix it there as well as delete here.

info[DeviceFields.APP_VERSION] = this.appInfo.localAppVersion;
}

info[DeviceFields.PLATFORM] = "server";
Copy link
Contributor

Choose a reason for hiding this comment

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

[minor] shouldn't this be node-server or js-server?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

### Using the SDK

#### Initialize the SDK
1. When your app or webpage is initialized, run the following code to initialize the Stitch SDK, replacing `<your-client-app-id>` with your Stitch application's client app ID:
Copy link
Contributor

Choose a reason for hiding this comment

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

[nit] app or webpage -> Node.js service

@coveralls
Copy link

Coverage Status

Coverage increased (+0.8%) to 82.535% when pulling e86e0a3 on edaniels:STITCH-1720 into 530f94c on mongodb:master.

Copy link
Contributor

@jsflax jsflax left a comment

Choose a reason for hiding this comment

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

Big PR, but LGTM! Good job on the FileStorage part

@edaniels edaniels merged commit dcd1cb7 into mongodb:master Jun 29, 2018
@edaniels edaniels deleted the STITCH-1720 branch June 29, 2018 15: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