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

MLIBZ-2762 Deprecate `loginWithAuthorizationCodeAPI()` method for automated authorization grant flow #208

Conversation

@yuliya-guseva
Copy link
Contributor

commented Dec 10, 2018

Description

The goal of the task is deprecate loginWithAuthorizationCodeAPI() method for automated authorization grant flow

Changes

Deprecated loginWithAuthorizationCodeAPI() in favor of a loginWithMIC() method

Tests

Instrumented

feat:
MLIBZ-2762 Deprecate `loginWithAuthorizationCodeAPI()` method for automated authorization grant flow
feat:
Fix comment
@codecov-io

This comment has been minimized.

Copy link

commented Dec 10, 2018

Codecov Report

❗️ No coverage uploaded for pull request base (indev@4276646). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff            @@
##             indev     #208   +/-   ##
========================================
  Coverage         ?   69.32%           
  Complexity       ?      578           
========================================
  Files            ?       41           
  Lines            ?     3162           
  Branches         ?      480           
========================================
  Hits             ?     2192           
  Misses           ?      807           
  Partials         ?      163
Impacted Files Coverage Δ Complexity Δ
.../main/java/com/kinvey/android/store/UserStore.java 67.7% <0%> (ø) 33 <0> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4276646...1ff16a6. Read the comment docs.

@vinaygahlawat
Copy link
Contributor

left a comment

For both loginWithAuthorizationCodeLoginPage and loginWithAuthorizationCodeAPI, instead of copy/pasting functionality, please have them call the appropriate overloaded loginWithMIC method. This was, the implementations are in loginWithMIC, and the deprecated methods now just call the new method.

Also, are there unit/instrumented tests that should change because we are deprecating these methods?

@yuliya-guseva

This comment has been minimized.

Copy link
Contributor Author

commented Dec 11, 2018

Ok. We have some tests for the old methods(about 4-5) . If old methods call the new overloaded method, new method will be covered by tests. Also we can duplicate this tests only for the new method

feat:
MLIBZ-2762 Deprecate `loginWithAuthorizationCodeAPI()` method for automated authorization grant flow
@vinaygahlawat
Copy link
Contributor

left a comment

Overall LGTM, but do we need to make unit test changes here to call LoginWithMIC, rather than the deprecated methods?

@b-stolyarov

This comment has been minimized.

Copy link
Collaborator

commented Jan 15, 2019

I think we do not need to make unit test changes and create new tests, because we have tests for old methods and they cover new method because now we call LoginWithMIC in old methods

@b-stolyarov b-stolyarov merged commit 6a98ecf into indev Jan 17, 2019

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

@b-stolyarov b-stolyarov deleted the feature/MLIBZ-2762_Give_feedback_Deprecate_loginWithAuthorizationCodeAPI()_method_for_automated_authorization_grant_flow branch Jan 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.