Skip to content
This repository has been archived by the owner on Feb 11, 2022. It is now read-only.

Remove Log class #19

Closed
blundell opened this issue Jun 4, 2014 · 12 comments
Closed

Remove Log class #19

blundell opened this issue Jun 4, 2014 · 12 comments
Assignees

Comments

@blundell
Copy link
Contributor

blundell commented Jun 4, 2014

Remove the Merlin Log class & add a dependency on NoTils

https://github.com/novoda/merlin/blob/master/core/src/main/java/com/novoda/merlin/Log.java

@ouchadam
Copy link
Contributor

do we want to add another dependency to this library? Would we be better off renaming Log to MerlinLog?

@blundell
Copy link
Contributor Author

Gradle is clever enough that adding an internal dependency to NoTils is fine.

@ouchadam
Copy link
Contributor

I don't want to pull in all those Notils methods though 😖

@blundell
Copy link
Contributor Author

Why not, what is the downside?

@xrigau
Copy link
Contributor

xrigau commented Jul 12, 2014

forcing libraries in libraries is not cool, it just adds more than what it's supposed. i.e. If I want to use Merlin it is because it does 1 thing and it works well, but I'm forced to use NoTils just because Merlin wants to log using the SimpleLog. What if I'm not gonna use anything in NoTils? I'd just have it there for a simple log, doesn't make much sense

@blundell
Copy link
Contributor Author

The idea is nobody knows you are using NoTils. I don't think your argument is valid, your saying no library can never use any help (another library) from anywhere.

We're already using keviswiki's HTTP client, what if we wanted to use Guava, or the Support Librarys or ViewPagerIndicator or Gson. Are you saying you'd want to roll your own json parser in every library? :-)

@ouchadam
Copy link
Contributor

Except you'll end up having to exclude notils if you start to run out of methods!
In this case we would only use the SimpleLog. I'm also planning to remove the HttpRequest library

@blundell
Copy link
Contributor Author

I can see what you're thinking its kind of a pre-optimisation tho. I
wouldn't do that extra work myself
On 12 Jul 2014 17:55, "Adam Brown" notifications@github.com wrote:

Except you'll end up having to exclude notils if you start to run out of
methods!
In this case we would only use the SimpleLog. I'm also planning to remove
the HttpRequest library


Reply to this email directly or view it on GitHub
#19 (comment).

@blundell
Copy link
Contributor Author

blundell commented Sep 9, 2014

@xrigau @ouchadam I am happy to go with your decision, if you choose now - Are we going to include NoTils for logging (& possibly further useage i.e. when we discussed this simple connection test) or will we rename Log to MerinLog ?

@ouchadam
Copy link
Contributor

ouchadam commented Sep 9, 2014

another solution would be to simply use Android.Log without the wrapper class

@ouchadam
Copy link
Contributor

ouchadam commented Sep 9, 2014

renaming the MerlinLog is the simplest fix

@xrigau
Copy link
Contributor

xrigau commented Sep 9, 2014

👍 for renaming

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

No branches or pull requests

4 participants