-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add AM/PM circles #2
base: master
Are you sure you want to change the base?
Conversation
First of all, thanks for the PR! Unfortunately, this library tracks a Google library, just making the necessary changes to make it usable in a library form. For this reason, I'm inclined to not accept this, though you are welcome to maintain a fork! |
https://android.googlesource.com/platform/frameworks/opt/datetimepicker/+/f41bcf49a4ab0b2f204533c757069cf2edfabc97/src/com/android/datetimepicker/time/AmPmCirclesView.java |
My bad! Will review it right away. |
compile 'com.android.support:support-v4:23.3.0' | ||
compile 'com.android.support:appcompat-v7:23.3.0' | ||
compile 'com.android.support:support-v4:24.0.0@aar' | ||
compile 'com.android.support:appcompat-v7:24.0.0@aar' |
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 @aar
on both of these? Shouldn't be needed?
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 lean dependencies.
You can run gradle task - [androidDependencies] and see the results. For use there is no different.
No @aar
Information:Gradle tasks [androidDependencies]
:DateTimePickerLibrary:androidDependencies
debug
+--- com.android.support:support-v4:24.0.0
| \--- LOCAL: internal_impl-24.0.0.jar
\--- com.android.support:appcompat-v7:24.0.0
+--- com.android.support:support-v4:24.0.0
| \--- LOCAL: internal_impl-24.0.0.jar
+--- com.android.support:support-vector-drawable:24.0.0
| \--- com.android.support:support-v4:24.0.0
| \--- LOCAL: internal_impl-24.0.0.jar
\--- com.android.support:animated-vector-drawable:24.0.0
\--- com.android.support:support-vector-drawable:24.0.0
\--- com.android.support:support-v4:24.0.0
\--- LOCAL: internal_impl-24.0.0.jar
With @aar
Information:Gradle tasks [androidDependencies]
:DateTimePickerLibrary:androidDependencies
debug
+--- com.android.support:support-v4:24.0.0
| \--- LOCAL: internal_impl-24.0.0.jar
\--- com.android.support:appcompat-v7:24.0.0
Could you improve the code parity with the original source? This PR uses different names, different code locations, etc, which will make it much harder to update in the future. |
Can you give me some suggestions about how to improve? Because I don't know what code style you prefer, or can you just ignore my PR use other way to do that? |
Sorry for the delay getting back to you. Workload has been increasing lately and it's been a bit hard to keep up. What I mean about code parity I'm not talking about my code style preference... just to try to have 1-on-1 parity of the code structure, naming and flow with the original source by Google. I can look more deeply into this during August, but I'm hoping my feedback is clearer this time around! |
No description provided.