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

Update MongoDbPatternLayoutAppender.java to save timestamp as a date type in mongodb #6

Closed
wants to merge 1 commit into from

Conversation

pramodransingh
Copy link
Contributor

adding code to save pattern layout timestamp in the mongodb as a Date type. Before it was getting saved as a string.

adding code to save pattern layout timestamp in the mongodb as a Date type. Before it was getting saved as a string.
@pramodransingh pramodransingh changed the title Update MongoDbPatternLayoutAppender.java Update MongoDbPatternLayoutAppender.java to save timestamp as a date type in mongodb Aug 8, 2018
@RobertStewart
Copy link
Owner

Thanks for the contribution, but I'd like to request a few changes before I merge it.

  1. I don't want to change the existing behavior of MongoDbPatternLayoutAppender, because this change will break compatibility for people already using it. Please create a new class named MongoDbPatternLayoutDateAppender that extends MongoDbPatternLayoutAppender and overrides append(). Since that means you would inherit only requiresLayout(), I'm fine if you either create an independent class or if you extract a protected method around where the bson variable is set and you override that.

  2. You are missing imports for org.apache.log4j.Layout and java.util.Date, which prevents the code from compiling.

  3. Please use the syntax conventions used in the rest of the code, e.g., a blank space before a ( and =.

Thanks!

@pramodransingh
Copy link
Contributor Author

pramodransingh commented Aug 13, 2018 via email

@RobertStewart
Copy link
Owner

Were you planning to update the pull request? I don't see any changes, yet.

@pramodransingh
Copy link
Contributor Author

I am closing this PR and creating new one

@pramodransingh
Copy link
Contributor Author

pramodransingh commented Aug 14, 2018 via email

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.

2 participants