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

Delete commons submodule and use library implementation (jitpack) #12

Merged
merged 1 commit into from Jan 9, 2019
Merged

Conversation

TacoTheDank
Copy link
Contributor

  • delete commons submodule and use library implementation (jitpack)
  • fix some English
  • reformat a bit of code

Signed-off-by: Taco SkytkRSfan3895@gmail.com

@TacoTheDank TacoTheDank changed the title delete commons submodule and use library implementation (jitpack) Delete commons submodule and use library implementation (jitpack) Jan 2, 2019
app/build.gradle Outdated
@@ -50,5 +51,5 @@ play {
}

dependencies {
implementation project(':plugin-commons')
implementation 'com.github.vanilla-music:vanilla-music-plugin-commons:v1.0.0'
Copy link
Member

Choose a reason for hiding this comment

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

It's 1.0.0, without "v". Sorry, all other plugins follow same scheme, can't change it for one library.

app/build.gradle Outdated
release {
zipAlignEnabled true
minifyEnabled true
Copy link
Member

Choose a reason for hiding this comment

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

What does this do? This won't enable proguard, will it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

minifyEnabled will, but zipAlignEnabled isn't needed because it's already set to true for release builds by default.

Copy link
Member

Choose a reason for hiding this comment

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

The thing is, I never tested this plugin with proguard, can you add useProguard false then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uhh sure, I can also just set minifyEnabled false for now.

app/build.gradle Outdated
@@ -3,13 +3,14 @@ apply plugin: 'com.github.triplet.play'

android {
compileSdkVersion 28
buildToolsVersion '28.0.3'
Copy link
Member

Choose a reason for hiding this comment

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

Not required. Without this it'll take latest one for compile sdk, which is what we need

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmmk

@@ -242,10 +242,10 @@ public boolean onOptionsItemSelected(MenuItem item) {
* extras from artwork response (if found) or with original intent
* </pre>
*/
private static boolean pluginInstalled(Context ctx, String pkgName) {
private static boolean pluginInstalled(Context ctx) {
Copy link
Member

Choose a reason for hiding this comment

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

Nope, this will add confusion. This function can possibly be reused for other plugins, so please leave it as it was.

build.gradle Show resolved Hide resolved
@TacoTheDank TacoTheDank closed this Jan 8, 2019
Signed-off-by: Taco <SkytkRSfan3895@gmail.com>
@TacoTheDank TacoTheDank reopened this Jan 8, 2019
@TacoTheDank
Copy link
Contributor Author

@Adonai was able to get around to it

@Kaned1as
Copy link
Member

Kaned1as commented Jan 9, 2019

Looks good to me, not sure why Travis doesn't like you.

@Kaned1as Kaned1as merged commit 5c7eff5 into vanilla-music:master Jan 9, 2019
@TacoTheDank
Copy link
Contributor Author

Travis never did like me much lol

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

Successfully merging this pull request may close these issues.

None yet

2 participants