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

implemented in-app update functionality #105

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

AdiAr11
Copy link

@AdiAr11 AdiAr11 commented Nov 4, 2021

Signed-off-by: AdiAr11 aditya2000arora@gmail.com

Description

Added in-app update feature

Impacted Issue number

Closes #85

Type of change

Enhancement

Code quality checklist

Just put an x in the [] where applicable.

  • I ran gradlew lint and found no new error related to my code in the report.
  • I have verified my changes on an emulator/ real device.
  • I have added proper comments where code is complex.

Signed-off-by: AdiAr11 <aditya2000arora@gmail.com>
@Ni3verma Ni3verma self-requested a review November 4, 2021 09:46
@Ni3verma Ni3verma added the enhancement Enhancing existing functionality label Nov 4, 2021
.idea/gradle.xml Outdated Show resolved Hide resolved
.idea/misc.xml Outdated Show resolved Hide resolved
.idea/vcs.xml Outdated Show resolved Hide resolved
Copy link
Owner

@Ni3verma Ni3verma left a comment

Choose a reason for hiding this comment

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

@AdiAr11 I have added some comments.. most of them are related to reverting some changes.
I will read the documentation and then review app update related code.
one more suggestion, please add Timber info logs wherever you think it will be useful.

@AdiAr11
Copy link
Author

AdiAr11 commented Nov 4, 2021

@AdiAr11 I have added some comments.. most of them are related to reverting some changes. I will read the documentation and then review app update related code. one more suggestion, please add Timber info logs wherever you think it will be useful.

I will make the required changes. Also, I haven't actually used Timber logs before, I'm still learning Android Development, sorry. I will look it up.
Thank you !!

This reverts commit a0dba18

Signed-off-by: AdiAr11 <aditya2000arora@gmail.com>
@AdiAr11 AdiAr11 closed this Nov 5, 2021
@AdiAr11 AdiAr11 deleted the feature/in_app_update branch November 5, 2021 09:35
@AdiAr11 AdiAr11 restored the feature/in_app_update branch November 5, 2021 09:36
@AdiAr11 AdiAr11 reopened this Nov 5, 2021
AdiAr11 and others added 3 commits November 5, 2021 15:19
Signed-off-by: AdiAr11 <aditya2000arora@gmail.com>
Signed-off-by: AdiAr11 <aditya2000arora@gmail.com>
Copy link
Owner

@Ni3verma Ni3verma left a comment

Choose a reason for hiding this comment

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

have added some comments for code placement.
also are you not running git hooks? step 7 in contibuting.md file?
your PR is failing with detekt,ktlint errors.

@AdiAr11
Copy link
Author

AdiAr11 commented Nov 5, 2021

have added some comments for code placement. also are you not running git hooks? step 7 in contibuting.md file? your PR is failing with detekt,ktlint errors.

I'm sorry but there was no check box for git hooks. I'll check and make the required changes

Signed-off-by: AdiAr11 <aditya2000arora@gmail.com>
# Conflicts:
#	app/src/main/res/values/strings.xml
Signed-off-by: AdiAr11 <aditya2000arora@gmail.com>
Copy link
Owner

@Ni3verma Ni3verma left a comment

Choose a reason for hiding this comment

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

while merging strings.xml, you accidently removed incoming changes
you had to merge : basically incoming changes + your changes

@AdiAr11
Copy link
Author

AdiAr11 commented Nov 8, 2021

while merging strings.xml, you accidently removed incoming changes you had to merge : basically incoming changes + your changes

oh, sorry..I'll correct this mistake

Signed-off-by: AdiAr11 <aditya2000arora@gmail.com>
@Ni3verma Ni3verma marked this pull request as draft November 9, 2021 05:13
@AdiAr11
Copy link
Author

AdiAr11 commented Nov 9, 2021

Please let me know if something else needs to be changed

}

private fun snackbarForCompleteUpdate() {
Snackbar.make(findViewById(R.id.content), "App update has been downloaded", Snackbar.LENGTH_INDEFINITE)
Copy link
Owner

Choose a reason for hiding this comment

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

take this also from string

Copy link
Owner

@Ni3verma Ni3verma left a comment

Choose a reason for hiding this comment

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

one small string hardcode issue
and currently you are always showing update dialog whenever it is available... like we discussed, keep some buffer... maybe ask after every 2/5 days.

@AdiAr11
Copy link
Author

AdiAr11 commented Nov 10, 2021

could you walk me through a little on how to go about asking for updating after 2/5 days?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancing existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request : Integrate In-app update
2 participants