-
Notifications
You must be signed in to change notification settings - Fork 23
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
Issue 2517: improve filestore pages generating with xsl #2588
base: main
Are you sure you want to change the base?
Issue 2517: improve filestore pages generating with xsl #2588
Conversation
Hi! Thank you for contributing! |
dd48920
to
f93205e
Compare
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.
Идея понятна, но надо разбить на более мелкие PR:
- xsl_render-либа с юниттестами
- поддержка дампа конфигов в XML помимо HTML
- PR с переводом service monitoring на XML+XSL
- PR с переводом tablet monitoring на XML+XSL
@@ -106,6 +106,13 @@ void DumpImpl( | |||
} | |||
} | |||
|
|||
template <typename T> | |||
TString DumpImpl(const T& value) { |
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.
{ не там
} \ | ||
using namespace NStorage::NTNodeWrapper; | ||
TNodeWrapper wrapper(root.AddChild("config_properties", " ")); | ||
#define VHOST_CONFIG_DUMP(name, ...) \ |
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.
$ echo -n '#define VHOST_CONFIG_DUMP(name, ...) \'|wc -c
81
@@ -8,10 +8,12 @@ SRCS( | |||
PEERDIR( | |||
cloud/filestore/libs/endpoint | |||
cloud/filestore/libs/service | |||
cloud/filestore/libs/client |
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.
не по алфавиту
@@ -129,6 +129,13 @@ void DumpImpl(const TVector<TString>& value, IOutputStream& os) | |||
} | |||
} | |||
|
|||
template <typename T> | |||
TString DumpImpl(const T& value) { |
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.
{ не там
|
||
<xsl:template match="root"> | ||
<xsl:choose> | ||
<xsl:when test="has_data"> |
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.
императивный стиль (if/else/for/etc.) в xslt - моветон, но лучше это обсудить в отдельном PR
#include "xml_document.h" | ||
|
||
namespace NCloud::NStorage::NTNodeWrapper { | ||
TNodeWrapper::TNodeWrapper(NXml::TNode root) : Root(root) {} |
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.
перед declaration / definition мы обычно пишем отбивку из /////...
@@ -0,0 +1,33 @@ | |||
#pragma once | |||
|
|||
#ifdef THROW |
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.
что за жуть, зачем это?
|
||
namespace NCloud::NStorage::NTNodeWrapper { | ||
|
||
class TNodeWrapper { |
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.
{ не там
TXslInitializer xslInit; | ||
} // namespace | ||
|
||
void NCloud::NStorage::NXSLRender::NXSLRender(const char* xsl, const NXml::TDocument& document, IOutputStream& out) { |
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.
длинная строка
#include "xml_document.h" | ||
|
||
namespace NCloud::NStorage::NXSLRender { | ||
void NXSLRender(const char* xsl, const NXml::TDocument& document, IOutputStream& out); |
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.
а почему NXSL, а не XSL?
#2517