-
Notifications
You must be signed in to change notification settings - Fork 36
Conversation
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.
Thanks for sending a pr. Just a few things more that you need to take care of .
Please fill in the pr template with the details mentioned. Also fix the failing build. Once you're done don't forget to squash the commits.
@yashk2000 |
.show(); | ||
break; | ||
case R.id.radio_pooja: | ||
binding.totalView.setVisibility(View.VISIBLE); | ||
flag = 1; | ||
Toast.makeText(getBaseContext(), getString(R.string.delete_pooja), Toast.LENGTH_LONG) | ||
Toast.makeText(getBaseContext(), getString(R.string.delete_pooja), Toast.LENGTH_SHORT) |
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.
Change to getBaseContext
instead of getApplicationContext
as this is that preferred way since this Context lives untill Application shuts down.
I am seeing so many unused changes please change that
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.
@Chromicle
Thanks for the review.
Can you please highlight the unused changes .
So that i can know the specifics.
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 also change to getBaseContext instead of getApplicationContext in MainActivity ?
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.
There are 59 file changes please change them
MyApplication/app/build.gradle
Outdated
android { | ||
compileSdkVersion 29 | ||
buildToolsVersion "29.0.0" | ||
defaultConfig { |
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.
This is not templeApp gradle file you created some thing application named MyApplicattion
Please revert all these
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.
@Chromicle
Yes , I overlooked this fault .
working on it.
effcf08
to
1e8cf29
Compare
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.
Have done the Requested modifications.
Awaiting for review and approval
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.
To many unnecessary changes in the idea files. Do not add the idea files while pushing your changes please.
3be36b0
to
11224f6
Compare
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.
@AnoopkRajan Also can you update the pr template.
884a941
to
42ba927
Compare
@AnoopkRajan what he ment is change in Github PR template not in |
gradle.properties
Outdated
# org.gradle.parallel=true |
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.
this is not a change. then why is it here. can you rectify that
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.
Reverted
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.
it is not yet reverted.
pull_request_template.md
Outdated
@@ -0,0 +1,14 @@ | |||
# Related Issue |
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.
this is not what I meant. You have to fill the PR template for the Pull Request
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 revert this file
@AnoopkRajan I meant change this according to the PR template |
e039fb5
to
54c0cd2
Compare
Have added the PR template. |
.github/PULL_REQUEST_TEMPLATE
Outdated
@@ -1,5 +0,0 @@ | |||
Fixed #[Add issue number here. If you do not solve the issue entirely, please change the message e.g. "First steps for issues #IssueNumber] |
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 delete this file? Revert this please.
@AnoopkRajan please revert changes to the pr template. Rest seems to be fine. |
yes .... i am working on it . |
Fix Travis build Update DeleteData.java Update InsertData.java Update ReadAllData.java Update ReadSingleData.java Update UpdateData.java intented Refix gradle build Modified Requested Changes removed deleted code reverted changes changes
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.
Thanks @AnoopkRajan 😄
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.
@AnoopkRajan great work
Thanks for doing everything with patience 👍
Thanks, @AnoopkRajan for the PR. 🙂 |
Fixed: #79
Changes: Toast duration has been reduced.