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

DDFileLogger: fixed isLogFile: method for timestamp log naming convention #190

Conversation

dvor
Copy link
Member

@dvor dvor commented Dec 6, 2013

Issue #189.

isLogFile: method wasn't processing DDLogFileNamingConventionTimestamp. This if fixed now.

Also I've changed timestamp format. In previous format colons were automatically converted to slashes / due to file naming convention - colons are forbidden in file name.

@dvor
Copy link
Member Author

dvor commented Dec 6, 2013

@bpoplauschi @rivera-ernesto Now logs have .txt file format. Maybe there is sense to change it to .log?

@rivera-ernesto
Copy link
Member

I think that's a good idea. That's what Apple does too.

Do you want to add that change here?

@@ -233,28 +233,59 @@ - (NSString *)logsDirectory

- (BOOL)isLogFile:(NSString *)fileName
{
// A log file has a name like "log-<uuid>.txt", where <uuid> is a HEX-string of 6 characters.
//
// For example: log-DFFE99.txt
Copy link
Member

Choose a reason for hiding this comment

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

In case we with to *.log the file prefix wouldn't be needed anymore.

Maybe something like MobileSafari 2013-12-03 17-14.log where you could easily distinguish logs.

Copy link
Member

Choose a reason for hiding this comment

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

I agree, this MobileSafari 2013-12-03 17-14.log looks right

Copy link
Member Author

Choose a reason for hiding this comment

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

This format looks good. But I think we should leave old formats for compatibility. Something like this:

typedef enum {
    // Application name followed by date.
    // Example: MobileSafari 2013-12-03 17-14.log
    DDLogFileNamingConventionDefault,

    // old file naming convetion, using UUID.
    // Example: log-DFFE99.txt
    DDLogFileNamingConventionUUID,

    // old file naming convention, using UTC time in RFC 3339 format.
    // Example: log-2013-12-20T15-20-30Z.txt
    DDLogFileNamingConventionTimestamp
} DDLogFileNamingConvention;

@rivera-ernesto
Copy link
Member

Sure, if it's not too much hassle.

On the other hand there's not really a compatibility issue that would really break by using a new (and better) naming convention.

Personally I would keep only the last one as it already has both:

  • A UUID (and a meaningful one) DDLogFileNamingConventionUUID.
  • A timestamp, doh DDLogFileNamingConventionTimestamp.

This is easy to update for client apps, and more important, easier to maintain and test for us!

@rivera-ernesto
Copy link
Member

Related to #174.

@dvor
Copy link
Member Author

dvor commented Dec 9, 2013

Well, then we can get rid of other formats and those enum. It simplifies the code.

The only thing that can be annoying for users is that DDLogFileNamingConvention would be removed and they'd have to modify their code.

@rivera-ernesto
Copy link
Member

That would just be a one-time change for them.

@dvor dvor closed this Dec 10, 2013
@dvor
Copy link
Member Author

dvor commented Dec 10, 2013

#191 replaces this.

@dvor dvor deleted the issue189--ddfilelogger-bug-with-timestamp-format branch December 10, 2013 07:51
@rivera-ernesto
Copy link
Member

About versioning. I just noticed that Pods-environent has:

// CocoaLumberjack
#define COCOAPODS_POD_AVAILABLE_CocoaLumberjack
// This library does not follow semantic-versioning,
// so we were not able to define version macros.
// Please contact the author.
// Version: 1.6.5.1.

// CocoaLumberjack/Core
#define COCOAPODS_POD_AVAILABLE_CocoaLumberjack_Core
// This library does not follow semantic-versioning,
// so we were not able to define version macros.
// Please contact the author.
// Version: 1.6.5.1.

// CocoaLumberjack/Extensions
#define COCOAPODS_POD_AVAILABLE_CocoaLumberjack_Extensions
// This library does not follow semantic-versioning,
// so we were not able to define version macros.
// Please contact the author.
// Version: 1.6.5.1.

@dvor
Copy link
Member Author

dvor commented Dec 18, 2013

@rivera-ernesto Will releasing new version with right tag fix this? Or we have to remove 1.6.5.1 tag (which could hurt some of users)?

If new version is enought, than we can release 1.7 with #191. Well, it could be 1.6.6, but as to me changing log file name format is MINOR change.

@bpoplauschi
Copy link
Member

@rivera-ernesto Good catch. This was far from ideal, but I think releasing a 1.7 would get us back in trend. This way it will be easier to release a patch with 1.7.x, if needed.

@rivera-ernesto
Copy link
Member

I would prefer keeping 1.6.x releases until we get some bigger changes.
That way we can also release more frequently (which is really important) without getting to 2.0 or 3.0 too fast (where the version is just a detail).

That said we should release now as #194 will require more work.

@bpoplauschi
Copy link
Member

I can go with both 1.6.6 or 1.7.0, but I think there are more arguments in favour of 1.7.0:

  • the feature set has changed significantly since 1.6.0, so we should show that in a minor release
  • between 1.6.5.1 and the next version, we changed the repo from robbiehanson/CocoaLumberjack to CocoaLumberjack/CocoaLumberjack which is pretty significant, even though we should not affect anyone using Cocoapods
  • the new log file naming convention is also a feature
  • all those show we have new functionality that is backwards compatible instead of just fixes for a patch
  • if by any chance we release 1.6.6 and we need to patch it, we could do 1.6.7 or 1.6.6.1 and both have their imperfections.

I'd suggest we start doing releases by the book.

@dvor
Copy link
Member Author

dvor commented Dec 19, 2013

without getting to 2.0 or 3.0 too fast

We don't have to release 2.0 at all. We can always go 1.10.0 and 1.11.0. ;)

@rivera-ernesto
Copy link
Member

Ok, 1.7.0 ;)

Can you please do it? I have something to finish here...

@bpoplauschi
Copy link
Member

Ok, will do this asap.

@bpoplauschi
Copy link
Member

Release 1.7.0 is ready, Cocoapods specs pull request: CocoaPods/Specs#6295

@dvor
Copy link
Member Author

dvor commented Dec 19, 2013

Great! Thanks @bpoplauschi

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.

None yet

3 participants