-
Notifications
You must be signed in to change notification settings - Fork 511
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
Format mgr:5min and mgr:60min as YAML #1796
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
/* | ||
* Copyright (C) 1996-2023 The Squid Software Foundation and contributors | ||
* | ||
* Squid software is distributed under GPLv2+ license and includes | ||
* contributions from numerous individuals and organizations. | ||
* Please see the COPYING and CONTRIBUTORS files for details. | ||
*/ | ||
|
||
#include "squid.h" | ||
#include "YamlDateTime.h" | ||
|
||
#include <ctime> | ||
#include <iomanip> | ||
#include <ostream> | ||
|
||
void | ||
Time::YamlDateTime::print(std::ostream &os) const | ||
{ | ||
// need to add fractions and timezone on top of this | ||
static const char *yaml_time_format = "%Y-%m-%d %H:%M:%S"; | ||
const auto tm = gmtime(&tv_.tv_sec); | ||
os << std::put_time(tm, yaml_time_format); | ||
os << '.' << std::setw(2) << (tv_.tv_usec / 10000) << 'Z'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why limit time to two digits?? Debugging often requires higher precision. Squid does not use two digits in most contexts IIRC. The chosen format does not specify precision AFAICT:
Please use all available digits. |
||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,38 @@ | ||||||
/* | ||||||
* Copyright (C) 1996-2023 The Squid Software Foundation and contributors | ||||||
* | ||||||
* Squid software is distributed under GPLv2+ license and includes | ||||||
* contributions from numerous individuals and organizations. | ||||||
* Please see the COPYING and CONTRIBUTORS files for details. | ||||||
*/ | ||||||
|
||||||
#ifndef SQUID_SRC_TIME_YAMLDATETIME_H | ||||||
#define SQUID_SRC_TIME_YAMLDATETIME_H | ||||||
|
||||||
#include <iosfwd> | ||||||
#include <sys/time.h> | ||||||
|
||||||
namespace Time { | ||||||
|
||||||
/// Output onto an ostream a yaml-formatted datetime string (UTC) | ||||||
/// see https://yaml.org/type/timestamp.html | ||||||
class YamlDateTime | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really need to put YamlDateTime inside Time namespace? The class name already has "Time". And Yaml. And Date. Why pick Time out of those three? The namespace itself is highly questionable. I would keep this class outside that namespace. |
||||||
{ | ||||||
public: | ||||||
YamlDateTime(const struct timeval &tv) : tv_(tv) {}; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please avoid implicit conversions by default:
Suggested change
|
||||||
void print(std::ostream &) const; | ||||||
|
||||||
private: | ||||||
const struct timeval tv_; | ||||||
}; | ||||||
|
||||||
} // namespace Time | ||||||
|
||||||
inline auto & | ||||||
operator<<(std::ostream &os, const Time::YamlDateTime &dt) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Foo::X operators should be declared in Foo. See commit 25ecffe. |
||||||
{ | ||||||
dt.print(os); | ||||||
return os; | ||||||
} | ||||||
|
||||||
#endif /* SQUID_SRC_TIME_YAMLDATETIME_H */ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ namespace Time | |
{ | ||
|
||
class Engine; | ||
class YamlDateTime; | ||
|
||
} // namespace Time | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not see why we should name this format, but if you insist on naming it, please do not make the variable static and modifiable (there is no reason for any of that).