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

Updated to Timber.Tree #3

Open
wants to merge 2 commits into
base: master
from
Open

Conversation

@varshneyjayant
Copy link

@varshneyjayant varshneyjayant commented Aug 12, 2015

  1. Timber.HollowTree was deprecated
  2. Added support for Appname
1. Timber.HollowTree  was deprecated
2. Added support for Appname
@@ -193,7 +250,7 @@ private String toJson(Level level, String message, Throwable t, Object... args)
* @param t throwable
* @param args message formatting arguments
*/
private void log(Level level, String message, Throwable t, Object... args) {
private void log(int level, String message, Throwable t, Object... args) {

This comment has been minimized.

@mostlyjason

mostlyjason Aug 20, 2015

@psquickitjayant why change from Level to int?

This comment has been minimized.

@varshneyjayant

varshneyjayant Aug 21, 2015
Author

@mostlyjason the new library Timber.Tree takes int as parameter

@tony19

This comment has been minimized.

Instead of breaking out the Level enum into individual int and String declarations, you should leverage the enum's ordinal() and toString() (see http://ideone.com/nM2JGB). Then, the only changes that would be needed are in toJson() and the overridden log().

@tony19

This comment has been minimized.

The existing API (where the tag is a separate function call) follows the Timber pattern more closely, and I'd like to keep it that way unless there's a compelling reason to change.

@tony19

This comment has been minimized.

Adding appName is an interesting idea, but it's a new feature that belongs in its own pull request.

@tony19

This comment has been minimized.

Update comment to match new function API

@tony19

This comment has been minimized.

remove comment

@varshneyjayant
Copy link
Author

@varshneyjayant varshneyjayant commented Aug 31, 2015

@tony19 We have improved the usage of enums

@varshneyjayant
Copy link
Author

@varshneyjayant varshneyjayant commented Aug 31, 2015

Removed extra code for tags but made tag as parameter to be passed in the constructor as tag is best way to create source groups in Loggly. https://www.loggly.com/docs/source-groups/

@varshneyjayant
Copy link
Author

@varshneyjayant varshneyjayant commented Aug 31, 2015

Do you want a separate PR as feature update for appName? @tony19

@varshneyjayant
Copy link
Author

@varshneyjayant varshneyjayant commented Aug 31, 2015

updated comments.

@@ -53,6 +54,43 @@ public LogglyTree(String token) {
handler = new LogglyClient.Callback() {
@Override
public void success() {

This comment has been minimized.

@darran-kelinske-fivestars

What do we need to do to get a new version of Timber supported? I can take over this PR if necessary.

@varshneyjayant
Copy link
Author

@varshneyjayant varshneyjayant commented Aug 19, 2020

@darren-kelinske-fivestars Please go ahead and raise a PR. Also, please directly contact the loggly github page. Thanks, Jayant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.