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

Make domain and data modules dependent on javax.inject instead of dagger 2 #116

Merged
merged 1 commit into from
Jul 3, 2016

Conversation

DmitriyZaitsev
Copy link
Contributor

Dagger 2 has implicit dependency:

+--- com.google.dagger:dagger:2.0.2
|    \--- javax.inject:javax.inject:1

Domain and Data modules don't depend on any class from the Dagger 2 API.
They need only @Inject annotation, so it makes no sense to have the entire library in classpath.

@Rainer-Lang
Copy link

@android10 Could this PR be merged? Is there a reason against this PR?

@android10
Copy link
Owner

@Rainer-Lang hold your horses. There is another PR being cook and after we finish a discussion I will test this and merge. Patience my friend. 😄

@Rainer-Lang
Copy link

@android10 I like horses :-D
Thanks for response.

@android10
Copy link
Owner

@Rainer-Lang haha nice! The PR is valid but give some time to merge it :)

@android10
Copy link
Owner

@DmitriyZaitsev can you squash the commits so I merge afterwards? Sorry for the delay on this.

@DmitriyZaitsev
Copy link
Contributor Author

@android10, do you insist on squashing? I believe it's better to have two "atomic" independent commits.

@android10
Copy link
Owner

@DmitriyZaitsev I like to have one commit per PR. It is easy when you check history on master and in case of a potential rollback you don't deal with "atomic" commits.

Also, by squashing you can keep all the atomic comments anyway.

@DmitriyZaitsev
Copy link
Contributor Author

@android10, done!

  • squashed commits;
  • rebased onto master

@android10 android10 merged commit c6a2132 into android10:master Jul 3, 2016
@android10
Copy link
Owner

@DmitriyZaitsev thanks for the contribution!

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.

4 participants