Support numeric times in time parser formatter #1254
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@repeatedly could you review this? |
repeatedly
reviewed
Oct 4, 2016
| @@ -199,6 +213,12 @@ def compat_parameters_formatter(conf) | |||
| # TODO: warn obsolete parameters if these are deprecated | |||
| attr = compat_parameters_copy_to_subsection_attributes(conf, FORMATTER_PARAMS) | |||
|
|
|||
| # TODO: make a utility method in TimeFormatter to handle these conversion | |||
There was a problem hiding this comment.
This comment is still needed? Hard to implement in TimeFormatter?
There was a problem hiding this comment.
time_as_epoch shouldn't be implemented in TimeFormatter, because it's now have time_type for unixtime, float (and string). It's just for compatibility for existing plugins.
repeatedly
reviewed
Oct 4, 2016
| @@ -199,6 +213,12 @@ def compat_parameters_formatter(conf) | |||
| # TODO: warn obsolete parameters if these are deprecated | |||
| attr = compat_parameters_copy_to_subsection_attributes(conf, FORMATTER_PARAMS) | |||
|
|
|||
| # TODO: make a utility method in TimeFormatter to handle these conversion | |||
| # copies of this code: plugin_helper/compat_parameters, compat/formatter_utils and here | |||
repeatedly
reviewed
Oct 4, 2016
| @@ -199,6 +213,12 @@ def compat_parameters_formatter(conf) | |||
| # TODO: warn obsolete parameters if these are deprecated | |||
| attr = compat_parameters_copy_to_subsection_attributes(conf, FORMATTER_PARAMS) | |||
There was a problem hiding this comment.
Off topic. Ruby has Module#attr method so avoiding same name is better.
|
Updated the change. |
|
@repeatedly No more comments? I'll merge later if so. |
|
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There's many use cases to parse/format times in unix time or floating point values.
It should be supported by built-in TimeParser/TimeFormatter and common configurations.
This change also implements timezone control in extracting timestamp placeholders in compat_parameters.