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

io.codetail library removal from the project and included as a dependency #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Seishin
Copy link

@Seishin Seishin commented Apr 25, 2016

The included library io.codetail was removed from the project and is added as a dependency in build.gradle.

}

apply from: 'bintray.gradle'
apply from: 'maven.gradle'
//apply from: 'bintray.gradle'
Copy link
Owner

Choose a reason for hiding this comment

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

These two scripts should be left in

Copy link
Author

Choose a reason for hiding this comment

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

Okay. I just commented them out because I could't build the app.

@Seishin
Copy link
Author

Seishin commented Apr 28, 2016

The main idea behind moving the CircularReveal library as a dependency is that it conflicts with other libs that use it when it's just included as a source library in the projects.

@gowong
Copy link
Owner

gowong commented Apr 28, 2016

Thanks for doing this!
It's definitely nicer to have CIrcularReveal handled by Gradle. I think the reason I didn't do this originally is because since CircularReveal needs Jitpack as a maven source, any project using this library would need to include this snippet in their gradle file as well (which might not be that big of a deal though):

maven {
    url "https://jitpack.io"
}

@Seishin
Copy link
Author

Seishin commented Apr 28, 2016

Yeah. But if you have two libs that use that library... They conflict. In my case I have to fork your lib and move the CircularReveal as a dependency in order to make it work fine with the project.

@gowong
Copy link
Owner

gowong commented Apr 28, 2016

It's possible to workaround that by excluding the CircularReveal dependency from the other libraries that are including it. For example:

compile("org.gradle.test.excludes:api:1.0") {
     exclude module: 'shared'
}

@jossiwolf
Copy link

+1 on this, this is really much needed - otherwise I can't use this library.

@gowong
Copy link
Owner

gowong commented Jul 18, 2016

@jossiwolf Have you tried excluding the module in your build.gradle? See my comment above yours

@jossiwolf
Copy link

jossiwolf commented Jul 19, 2016

@gowong Yes, I have.

compile('com.gordonwong:material-sheet-fab:1.2.1') {
        exclude module: 'io.codetail.widget'
        exclude module: 'io.codetail.animaton'
}

This doesn't work. Doesn't work with group: 'io.codetail' or module: 'io.codetail' either.

I think exclude only works for Gradle dependencies - you haven't added CircularReveal via Gradle which is why we can't exclude it:

+--- com.gordonwong:material-sheet-fab:1.2.1
|    \--- com.github.asyl.animation:arcanimator:1.0.0
|         \--- com.nineoldandroids:library:2.4.0

Edit: I included Seishin's fork instead now, which has Circular Reveal as dependency. It uses version 1.3.1, but I'm using version 2.0.1. Exclude works here:

compile('com.github.Seishin:material-sheet-fab:+') {
        exclude module: 'io.codetail'
}
compile('com.github.ozodrukh:CircularReveal:2.0.1') {
        transitive = true;
}
+--- com.github.Seishin:material-sheet-fab:+ -> v1.2.2
|    +--- com.github.asyl.animation:arcanimator:1.0.0
|    |    \--- com.nineoldandroids:library:2.4.0
|    \--- com.github.ozodrukh:CircularReveal:1.3.1 -> 2.0.1
+--- com.github.ozodrukh:CircularReveal:2.0.1

As you see, it is a necessity to include any library possible via gradle because it gives more flexibility. If you already include jitpack.io as repository in your libraries build.gradle, the user doesn't have to include it by himself.

@gowong
Copy link
Owner

gowong commented Jul 19, 2016

@jossiwolf
You're right, the dependency has to be in gradle, which is why I suggested to exclude Circular Reveal from the other libraries.

It's a temporary workaround that likely won't work in all cases. I'm not sure when I'll have time to do this so feel free to submit a PR (this current one has comments that need to be addressed)

@jossiwolf
Copy link

Sure, will do.

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

3 participants