{"action":"created","issue":{"url":"https://api.github.com/repos/tuskyapp/Tusky/issues/554","repository_url":"https://api.github.com/repos/tuskyapp/Tusky","labels_url":"https://api.github.com/repos/tuskyapp/Tusky/issues/554/labels{/name}","comments_url":"https://api.github.com/repos/tuskyapp/Tusky/issues/554/comments","events_url":"https://api.github.com/repos/tuskyapp/Tusky/issues/554/events","html_url":"https://github.com/tuskyapp/Tusky/pull/554","id":308287037,"number":554,"title":"Add Dagger","user":{"login":"charlag","id":3099142,"avatar_url":"https://avatars0.githubusercontent.com/u/3099142?v=4","gravatar_id":"","url":"https://api.github.com/users/charlag","html_url":"https://github.com/charlag","followers_url":"https://api.github.com/users/charlag/followers","following_url":"https://api.github.com/users/charlag/following{/other_user}","gists_url":"https://api.github.com/users/charlag/gists{/gist_id}","starred_url":"https://api.github.com/users/charlag/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/charlag/subscriptions","organizations_url":"https://api.github.com/users/charlag/orgs","repos_url":"https://api.github.com/users/charlag/repos","events_url":"https://api.github.com/users/charlag/events{/privacy}","received_events_url":"https://api.github.com/users/charlag/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-03-24T18:28:38Z","updated_at":"2018-03-24T19:08:18Z","closed_at":null,"author_association":"COLLABORATOR","pull_request":{"url":"https://api.github.com/repos/tuskyapp/Tusky/pulls/554","html_url":"https://github.com/tuskyapp/Tusky/pull/554","diff_url":"https://github.com/tuskyapp/Tusky/pull/554.diff","patch_url":"https://github.com/tuskyapp/Tusky/pull/554.patch"},"body":"So, this should've been a simple task but I ended up changing much more than I wanted. This PR adds Dagger Dependency Injection with Android plugins which should significantly improve architecture and testability in the future. For now it's mostly used for the networking stuff because it's the most obvious thing but it should also be used for the DB/Account manger and probably and more. Adding new modules as well as new screens should be trivial.\r\n\r\nWhat we gain from this:\r\n* Shared Gson caches, OkHttp pools/caches, Retrofit instances which should improve performance\r\n* Testing things much simpler when they don't look for dependencies through static fields\r\n* Better architecture - less coupling, less responsibilities\r\n\r\nWhat we've lost:\r\n* I deliberately omitted dispatcher canceling. Fragments which want canceling already add calls to the list and then cancel so I felt that it was redundant (in most other places this behavior is counter-intuitive).\r\n\r\nOther changes:\r\n* We don't create many Retrofit instances (which is not cheap because of the reflection) so we change domain URL dynamically. This has a nice side effects: it is not easy to implement \"boost as\" or \"follow as\" actions, because we can add header to the request which will tell `AuthInterceptor` to use specified account. Interceptor can strip this header afterwards (this technique was described by Jack Wharton - one of the OkHttp/Retrofit creators)\r\n* I couldn't get rid of `ServiceLocator` ugliness because it is used in the `BaseActivity` and we cannot inject there (and I don't want to inject it in _every_ activity). I'm not sure why this check is there, maybe we don't really need it, maybe we should find some other way.\r\n* We also cannot inject in `SFragment` really to I used this as a chance to move some of the functionality which doesn't belong to Fragments anyway to the separate class. It is not super clear abstraction but it should work, I think we could move even more things from `SFragment`. Subclassing should take place when there's a shared behavior, not when there's a shared code and you don't know where to put it. I still had to inject new things in every fragment but I think it's still a step in the right direction.\r\n* I've also added missing licenses\r\n\r\nI really don't like that we have many activities. Because of this this PR is much bigger.\r\nWhile it seems redundant to add API to every activity/fragment manually it uncovers hidden dependencies. If dependency is hidden behind the parent it doesn't mean it's not there. Injecting != using constructor but it has similar effect."},"comment":{"url":"https://api.github.com/repos/tuskyapp/Tusky/issues/comments/375917208","html_url":"https://github.com/tuskyapp/Tusky/pull/554#issuecomment-375917208","issue_url":"https://api.github.com/repos/tuskyapp/Tusky/issues/554","id":375917208,"user":{"login":"connyduck","id":10157047,"avatar_url":"https://avatars1.githubusercontent.com/u/10157047?v=4","gravatar_id":"","url":"https://api.github.com/users/connyduck","html_url":"https://github.com/connyduck","followers_url":"https://api.github.com/users/connyduck/followers","following_url":"https://api.github.com/users/connyduck/following{/other_user}","gists_url":"https://api.github.com/users/connyduck/gists{/gist_id}","starred_url":"https://api.github.com/users/connyduck/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/connyduck/subscriptions","organizations_url":"https://api.github.com/users/connyduck/orgs","repos_url":"https://api.github.com/users/connyduck/repos","events_url":"https://api.github.com/users/connyduck/events{/privacy}","received_events_url":"https://api.github.com/users/connyduck/received_events","type":"User","site_admin":false},"created_at":"2018-03-24T19:08:18Z","updated_at":"2018-03-24T19:08:18Z","author_association":"OWNER","body":"omneg this is wonderful <3\r\nwill have a look at it later or tomorrow"}}
{"actor":{"display_login":"connyduck"}}