-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix for issue #1037 #1086
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
Fix for issue #1037 #1086
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1086 +/- ##
======================================
Coverage 4.11% 4.11%
======================================
Files 114 114
Lines 5298 5298
Branches 501 501
======================================
Hits 218 218
Misses 5065 5065
Partials 15 15
Continue to review full report at Codecov.
|
@mithlesh4257 Any particular reason for opening a new PR for the same issue? IMO the discussions get lost if the same PR is not maintained. |
Actually @maskaravivek, I don't know how to manage this huge repository multiple file. What I mean to say I don't know how to add another commit to same PR. |
After committing your changes locally, just do a git push <branch_name>. It will automatically add that commit to the PR. :) |
Thanks @maskaravivek. Now is this PR according? |
import timber.log.Timber; | ||
|
||
import static android.view.MotionEvent.ACTION_DOWN; | ||
import static android.view.MotionEvent.ACTION_UP; | ||
|
||
public class SingleUploadFragment extends CommonsDaggerSupportFragment { | ||
public class SingleUploadFragment extends DaggerFragment { | ||
|
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.
Why did you changed this?
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 haven't changed it, it's automatically changed.
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.
Please undo it. Usage of DaggerFragment
would cause crashes.
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.
Sure I will remove it @maskaravivek
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.
@maskaravivek, I had deleted the previous Commons forked repository due to some bugs and forked again. Can I make another PR as through this I'm unable to change my files? Please help @maskaravivek !
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.
Should I make another PR that is in that I just have changed "upload/SingleUploadFragment.java" file as I had deleted the previously forked repo, I'm not also able to change this file directly from this PR.
@maskaravivek, @neslihanturan Please help!
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.
Or I have one solution for it, May you edit this @maskaravivek , @neslihanturan?
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.
@mithlesh4257 you can fork it again, and make a fresh PR don't worry. I am closing this. Please follow this document before https://github.com/commons-app/apps-android-commons/wiki/Setting-up-dev-enviroment by changing "maskaravivek" username with yours. And please make sure all of the steps we discussed here are covered before preparing PR. Make sure you checked your commits and added meaningful commit messages before pushing.
Besides, if you want to leave this task for someone else to handle, please notify us.
|
||
|
||
|
||
*/ | ||
|
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.
For what this empty lines stands for?
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'm removing it.
Fix for issue #1037
Explicitly show that the license text is a link.
@psh, @misaochan Kindly review it.