App install Refactor #2704
Merged
App install Refactor #2704
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@damagatchi retest this please |
|
@damagatchi retest this please |
avazirna
previously approved these changes
Sep 18, 2023
There was a problem hiding this comment.
@shubham1g5 this looks good to me, I just left a minor comment
|
|
||
| public static CommCareApp startAppInstallAsync(boolean shouldSleep, int taskId, CommCareTaskConnector connector, | ||
| String installRef) { | ||
| CommCareApp ccApp = getNewCommCareApp(); |
There was a problem hiding this comment.
Nit: Can we consider renaming this to something more descriptive, something like createNewCommCareApp, I see that it was renamed from getCommCareApp but I see the use of get as misleading as it seems that we are retrieving an existing element.
OrangeAndGreen
previously approved these changes
Sep 18, 2023
shubham1g5
dismissed stale reviews from OrangeAndGreen and avazirna
via
58c52ca
|
@damagatchi retest this please |
OrangeAndGreen
approved these changes
Sep 20, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Move app install handling to utils class for easy accessibility to Connect Classes.
Best reviewed by commit
Safety Assurance
Automated test coverage
All our tests test for App installs
Safety story
minor refactor that seemingly is no-op with a good test coverage around refactored code.