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

Changing IAccount Interface to AccountInfo #1789

Merged
merged 18 commits into from Jun 23, 2020

Conversation

pkanher617
Copy link
Contributor

This PR changes the account object returned to the user from the IAccount interface to the AccountInfo type.

It also refactors the code to remove the CacheHelpers.ts file, and removes the matches logic from the CacheManager.ts file.

Serializer needs to figure out how to implement swap and rename with strict type checks turned on.
@github-actions github-actions bot added msal-browser Related to msal-browser package msal-common Related to msal-common package msal-node Related to msal-node package labels Jun 17, 2020
Copy link
Contributor

@sangonzal sangonzal left a comment

Choose a reason for hiding this comment

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

:shipit:

@@ -12,10 +13,11 @@ export class EntitySerializer {
* @param key
*/
static mapAccountKeys(accCache: AccountCache, key: string): object {
Copy link
Contributor

Choose a reason for hiding this comment

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

Some extra stuff in this file that I think is no longer needed (commented out)

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 for removing unneeded comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is being fixed by @sameerag.

Copy link
Collaborator

@tnorling tnorling left a comment

Choose a reason for hiding this comment

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

2 comments, overall looks fine once those have been addressed

@@ -12,10 +13,11 @@ export class EntitySerializer {
* @param key
*/
static mapAccountKeys(accCache: AccountCache, key: string): object {
Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 for removing unneeded comments

@pkanher617 pkanher617 changed the base branch from msal-common-unifiedCache-update to dev June 18, 2020 21:22
@sameerag sameerag changed the base branch from dev to serialize-deserialize June 20, 2020 00:31
@pkanher617 pkanher617 changed the base branch from serialize-deserialize to dev June 23, 2020 03:51
@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 77.514% when pulling eb29bfa on msal-common-account-interface into 5330176 on dev.

@pkanher617 pkanher617 merged commit f842023 into dev Jun 23, 2020
@sameerag sameerag deleted the msal-common-account-interface branch July 20, 2020 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
msal-browser Related to msal-browser package msal-common Related to msal-common package msal-node Related to msal-node package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants