Skip to content

Conversation

@adamchel
Copy link
Contributor

@adamchel adamchel commented Feb 8, 2019

This basically adds new methods to the StitchAuthListener interface, enriching its use cases.

About 1,000 of the lines of code here are 300 lines of integration tests, copied across the three SDKs. The integration tests are pretty much the same across the platforms so it's probably worth it to only just look through one of the integration test files.

@adamchel adamchel requested review from jsflax and tkaye407 February 11, 2019 20:31
@coveralls
Copy link

coveralls commented Feb 11, 2019

Coverage Status

Coverage increased (+0.4%) to 77.273% when pulling 60c8dd9 on adamchel:STITCH-2485 into 8f121e1 on mongodb:master.

Copy link
Contributor

@tkaye407 tkaye407 left a comment

Choose a reason for hiding this comment

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

LGTM. Curious what your thoughts are on not actually having onAuthEventI() in the auth methods like swtich() and login() but rather whenever a switch() or a login() event "kind" come in you issue a legacy auth event as well? Could be done in dispatchAuthEvent()

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.

LGTM bar a couple style nits

@adamchel adamchel merged commit 54dcb45 into mongodb:master Feb 12, 2019
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