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

ExoPlayer 2 - Fix build by removing notils #45

Merged
merged 3 commits into from
Jun 9, 2017

Conversation

juankysoriano
Copy link
Contributor

Problem

We are having a lot of issues because of the notils support library versions clashing with libraries support versions.

Solution

We are not using notils a lot, instead I have "ported" the Logging function to an utils.Log class

Test(s) added

Modified some existing tests

Screenshots

No UI changes

Paired with

Nobody

import java.io.PrintWriter;
import java.io.StringWriter;

public abstract class Log {
Copy link
Contributor

Choose a reason for hiding this comment

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

we should rename this so that clients of the library don't get spammed with Log classes 🍡 NoPlayerLog?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was going to remove it, named it Log to make it easier the migration and forgot to rename it afterwards. Good point.

@jszmltr jszmltr self-assigned this Jun 9, 2017
@jszmltr jszmltr self-requested a review June 9, 2017 14:25
@rock3r rock3r requested review from rock3r and removed request for jszmltr June 9, 2017 14:25
@rock3r rock3r self-assigned this Jun 9, 2017
import java.io.PrintWriter;
import java.io.StringWriter;

public abstract class NoPlayerLog {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is basically a quick port of what we have in notils

Copy link
Contributor

@rock3r rock3r left a comment

Choose a reason for hiding this comment

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

One nit then g2g

if (!isEnabled) {
return;
}
android.util.Log.e("No-Player", logMessage(msg, throwable));
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit extract constant for the tag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

Copy link
Contributor

@rock3r rock3r left a comment

Choose a reason for hiding this comment

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

💯

@rock3r rock3r merged commit 048af1e into exo-player-2 Jun 9, 2017
@rock3r rock3r deleted the exoplayer2-remove-notils branch June 9, 2017 14:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants