-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Shift contributions to use Room DB #3324
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
Merged
neslihanturan
merged 32 commits into
commons-app:master
from
ashishkumar468:refactor/contributions-to-room
Mar 9, 2020
Merged
Changes from all commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
9e0b8f0
Part of #3127
ashishkumar468 d37148e
Sync with master
ashishkumar468 3d54774
Save and Fetch contributions via RoomDAO
ashishkumar468 899885c
Bugfixes, fixed test cases, injected schedulers for ContributionsPres…
ashishkumar468 ac33ee6
removed stetho
ashishkumar468 d31fbfc
Fixed ReviewHelperTest cases
ashishkumar468 5e060bd
Fixed test cases in DeleteHelperTest
ashishkumar468 85d99a0
Fetch all contributions [TODO add pagination to use this, maybe later…
ashishkumar468 37f17b6
Update Schema false in AppDatabase
ashishkumar468 509ea43
removed parameter from fetchControbutions
ashishkumar468 c8962a2
Merge branch 'master' into refactor/contributions-to-room
ashishkumar468 1424776
Added logs for fetch contributions
ashishkumar468 6a52241
Fixed test case ContributionsPresenter
ashishkumar468 e7140e1
Added an autogenerate primary key, submit save contributions on executor
ashishkumar468 2714df2
fixed getItemAtPosition
ashishkumar468 9d99527
MainActivity Config changes +=orientation
ashishkumar468 52c086f
BugFixes
ashishkumar468 adaa33c
Remove un-nescessary null check on contributions in ContributionsList…
ashishkumar468 c347200
removed ContributionsListFragment [not-implemeted]
ashishkumar468 adb6d70
Review suggested changes
ashishkumar468 8e33730
wip
ashishkumar468 3d91895
delete existing contributions table (from the existing db) on upgrade
ashishkumar468 0d87140
remove un-nescessary null checks in test classes
ashishkumar468 3081989
shifted media to be a local variable in ReviewHelperTest
ashishkumar468 0c4c783
removed captured folder
ashishkumar468 2985b1c
Dispose composite disposables in UploadService
ashishkumar468 faf9f41
replaced size check with isEmpty ContributionsPresenter
ashishkumar468 48c998b
transform saveContributions to a Completable
ashishkumar468 5d484c1
Addressed comments in review
ashishkumar468 dc4d76a
Provide Gson to Converters from the CommonsApplicationComponent
ashishkumar468 7f1c82b
use static method instead of field instead of static field to provide…
ashishkumar468 1181a81
Modified gitignore to exclude captures/*
ashishkumar468 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
versions should be specified in
gradle.properteries
and be in CONSTANT_CASEThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could not find any such thing in the developer docs, based on the project scope it could be in the project's build.gradle or app's build.gradle, can you share relevant doc links for the same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am following the example of the project. I don't see any other dependency declared like this but I do see leakcanary, dagger, kotlin & butterknife declared in gradle.properties.
It also just so happens that gradle.properties is the best place to put them, particularly in a multi module project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seannemann21 AFAIK, for multi-module projects, project-level build.gradle should be the place for such things
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, best practise advice is to configure your projects using a precompiled script or a plugin in your buildsrc folder. The best place to keep your version numbers is in gradle.properties, buildsrc is a bit annoying becuse any change in there cause a recompilation of the entire project.
Use buildsrc
Proper versioning and great library
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, well that would need pulling out other dependencies from project build.gradle and app build.gradle as well, right?, Should we not create a separate issue for the same and take it from there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps, but I am reviewing this addition. If you define a version put it in gradle.properties with the rest of them, as is evidently project style.