-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Link cognito sdk #182
Link cognito sdk #182
Conversation
mlabieniec
commented
Jan 23, 2018
•
edited
Loading
edited
- migrate amazon-cognito-js sdk into packages
- remove dependency on js aws-sdk on for amazon-cognito-js
- mutable custom attributes, Fixes Updating mutable user attributes #18
- yarn workspaces
- travis update for yarn / workspaces
- validation data, Fixes Allow SignUp with validation data #175
- Adds a podspec for CocoaPods support, (merge pr from Adds a podspec for CocoaPods support amazon-archives/amazon-cognito-identity-js#644)
…into link-cognito-sdk
Codecov Report
@@ Coverage Diff @@
## master #182 +/- ##
==========================================
+ Coverage 92.08% 92.24% +0.15%
==========================================
Files 52 52
Lines 2402 2424 +22
Branches 480 486 +6
==========================================
+ Hits 2212 2236 +24
+ Misses 181 179 -2
Partials 9 9
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
package.json
Outdated
], | ||
"repository": { | ||
"type": "git", | ||
"url": "https://github.com/aws/aws-amplify.git" | ||
}, | ||
"author": "Amazon Web Services", | ||
"license": "Apache 2.0", | ||
"license": "Apache", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use SPDX identifiers here?
Hi 👋 Why was the sub claim explicitly filtered out of the results? Seems the decision should be left up to the user on how/if they want to use that claim or am I missing something? Maybe a security concern of sorts? Or maybe I'm making a bad assumption? It seems like an intuitive UUID to use as a primary key in a dynamodb table, for example, and if that's valid, then having it available in the client would be necessary for, say, an AppSync graphql mutation or query. TIA for explaining the decision! |
* add cognito js lib and update references * update cognito lib * update node version and package deps * update cognito sdk and auth test * update travis commands * update react tests * update travis * update with yarn workspaces and modules * update with yarn workspaces and modules * add lint to test * update test cmd * update test cmd * update test cmd * update build cmds * update concurrency with bootstrap * set yarn workspaces * update travis * update deps, yarn should use local * remove workspaces * update workspaces * add build before test for workspaces * update travis * remove package-lock.json * update rn package * update deps on react native * update user attributes * update user attributes * fix variable * update docs * for rn * core change * Update package.json * Adds a podspec for CocoaPods support
This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs. Looking for a help forum? We recommend joining the Amplify Community Discord server |