You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2020/09/14 06:17:00 UTC
[GitHub] [logging-log4j2] gengyuanzhe opened a new pull request #419: LOG4J2-2889
gengyuanzhe opened a new pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419
HtmlLayout support datePattern and timezone
Submitted by: gengyuanzhe <ge...@gmail.com>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] gengyuanzhe commented on pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
gengyuanzhe commented on pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#issuecomment-695747057
> By the way, if you'd like this feature in 2.x and not just 3.x, can you file a PR to the `release-2.x` branch?
Hi, I'm trying to apply these changes to `release-2.x` branch, but encountered two problems.
1. When I tried to modify `org.apache.logging.log4j.core.layout.HtmlLayout#createLayout`, either by deleting or adding arguments, it was blocked by **revapi-maven-plugin**. Should I change this factory method? As I know, when create plugins, the `@PluginBuilderFactory` has a higher priority than `@PluginFactory`. Just modifying the Builder, it will work.
2. For `release-2.x`, the docs are **xml** or **xml.vm** format, rathan than asciidoc. Can you tell me how to convert them to html?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] gengyuanzhe edited a comment on pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
gengyuanzhe edited a comment on pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#issuecomment-692845402
@rgoers @jvz
Documentation is updated, and fix a timezone bug of DatePatternConverterTest. Please help review and merge, thank you
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] jvz commented on pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
jvz commented on pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#issuecomment-695353407
By the way, if you'd like this feature in 2.x and not just 3.x, can you file a PR to the `release-2.x` branch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] gengyuanzhe commented on a change in pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
gengyuanzhe commented on a change in pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#discussion_r491272078
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/layout/HtmlLayout.java
##########
@@ -345,14 +355,17 @@ public static HtmlLayout createLayout(
@PluginAttribute String contentType,
@PluginAttribute(defaultString = "UTF-8") final Charset charset,
@PluginAttribute String fontSize,
- @PluginAttribute(value = "fontName", defaultString = DEFAULT_FONT_FAMILY) final String font) {
+ @PluginAttribute(value = "fontName", defaultString = DEFAULT_FONT_FAMILY) final String font,
Review comment:
OK, this method has been deleted and test OK. Please review again.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] gengyuanzhe commented on a change in pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
gengyuanzhe commented on a change in pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#discussion_r491272078
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/layout/HtmlLayout.java
##########
@@ -345,14 +355,17 @@ public static HtmlLayout createLayout(
@PluginAttribute String contentType,
@PluginAttribute(defaultString = "UTF-8") final Charset charset,
@PluginAttribute String fontSize,
- @PluginAttribute(value = "fontName", defaultString = DEFAULT_FONT_FAMILY) final String font) {
+ @PluginAttribute(value = "fontName", defaultString = DEFAULT_FONT_FAMILY) final String font,
Review comment:
This method has been deleted and test OK. Please review again.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] gengyuanzhe commented on pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
gengyuanzhe commented on pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#issuecomment-700624599
@jvz Hi Matt, it has been two weeks. When will this PR be merged?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] gengyuanzhe edited a comment on pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
gengyuanzhe edited a comment on pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#issuecomment-695747057
> By the way, if you'd like this feature in 2.x and not just 3.x, can you file a PR to the `release-2.x` branch?
Hi, I'm trying to apply these changes to `release-2.x` branch, but encountered two problems.
1. When I tried to modify `org.apache.logging.log4j.core.layout.HtmlLayout#createLayout`, either by deleting or adding arguments, it was blocked by **revapi-maven-plugin**. Should I change this factory method? As I know, when create plugins, the `@PluginBuilderFactory` has a higher priority than `@PluginFactory`. Just modifying the Builder, it will work. There are trade-offs between API compatibility and consistency of the two factory methods(`createLayout` and `newBuilder`)
2. For `release-2.x`, the docs are **xml** or **xml.vm** format, rathan than asciidoc. Can you tell me how to convert them to html ? I need check my changes.
Thank you for help.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] gengyuanzhe commented on a change in pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
gengyuanzhe commented on a change in pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#discussion_r491272078
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/layout/HtmlLayout.java
##########
@@ -345,14 +355,17 @@ public static HtmlLayout createLayout(
@PluginAttribute String contentType,
@PluginAttribute(defaultString = "UTF-8") final Charset charset,
@PluginAttribute String fontSize,
- @PluginAttribute(value = "fontName", defaultString = DEFAULT_FONT_FAMILY) final String font) {
+ @PluginAttribute(value = "fontName", defaultString = DEFAULT_FONT_FAMILY) final String font,
Review comment:
OK, this method has been deleted. Please review again.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] gengyuanzhe commented on pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
gengyuanzhe commented on pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#issuecomment-700817139
OK, I see. I've been wondering why you didn't commit any codes these days. :rofl:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] jvz commented on pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
jvz commented on pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#issuecomment-695817855
The failed tests are flaky from before and unrelated to this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] gengyuanzhe edited a comment on pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
gengyuanzhe edited a comment on pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#issuecomment-695798876
@jvz
All of the changes have been applied to branch `release-2.x` with https://github.com/apache/logging-log4j2/pull/423.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] jvz commented on a change in pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
jvz commented on a change in pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#discussion_r491213545
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/layout/HtmlLayout.java
##########
@@ -345,14 +355,17 @@ public static HtmlLayout createLayout(
@PluginAttribute String contentType,
@PluginAttribute(defaultString = "UTF-8") final Charset charset,
@PluginAttribute String fontSize,
- @PluginAttribute(value = "fontName", defaultString = DEFAULT_FONT_FAMILY) final String font) {
+ @PluginAttribute(value = "fontName", defaultString = DEFAULT_FONT_FAMILY) final String font,
Review comment:
This factory method isn't needed anymore since the builder class is present.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] gengyuanzhe edited a comment on pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
gengyuanzhe edited a comment on pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#issuecomment-695747057
> By the way, if you'd like this feature in 2.x and not just 3.x, can you file a PR to the `release-2.x` branch?
Hi, I'm trying to apply these changes to `release-2.x` branch, but encountered a problem.
When I tried to modify `org.apache.logging.log4j.core.layout.HtmlLayout#createLayout`, either by deleting or adding arguments, it was blocked by **revapi-maven-plugin**. Should I change this factory method? As I know, when create plugins, the `@PluginBuilderFactory` has a higher priority than `@PluginFactory`. Just modifying the Builder, it will work. There are trade-offs between API compatibility and consistency of the two factory methods(`createLayout` and `newBuilder`)
Thank you for help.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] gengyuanzhe commented on pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
gengyuanzhe commented on pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#issuecomment-691847601
@jvz Please help review and merge this pr. Solve [LOG4J2-2889](https://issues.apache.org/jira/browse/LOG4J2-2889) and [LOG4J2-2714](https://issues.apache.org/jira/browse/LOG4J2-2714), with the unit tests. As the function of HtmlLayout is extended, should I update the [docs](https://logging.apache.org/log4j/2.x/manual/layouts.html#HTMLLayout)?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] gengyuanzhe commented on pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
gengyuanzhe commented on pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#issuecomment-695798876
@jvz
All of the changes have been handled for branch `release-2.x` with https://github.com/apache/logging-log4j2/pull/423.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] gengyuanzhe commented on pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
gengyuanzhe commented on pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#issuecomment-695801712
@jvz
In addition, I would like to know why your commit step is a success but mine is a failure for step `Maven/Build (Ubuntu-latest)`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] gengyuanzhe commented on pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
gengyuanzhe commented on pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#issuecomment-695798876
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] gengyuanzhe commented on pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
gengyuanzhe commented on pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#issuecomment-692845402
@rgoers @jvz
Documentation is updated, and fix a timezone bug of DatePatternConverterTest. Please review and merge.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] jvz merged pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
jvz merged pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] gengyuanzhe edited a comment on pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
gengyuanzhe edited a comment on pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#issuecomment-695747057
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] jvz commented on pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
jvz commented on pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#issuecomment-700790163
I was on vacation the past two weeks actually ;)
I'll be merging this within a day or two. Definitely before the next release.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] jvz commented on pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
jvz commented on pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#issuecomment-695817855
The failed tests are flaky from before and unrelated to this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] rgoers commented on pull request #419: [LOG4J2-2889] HtmlLayout support datePattern and timezone
Posted by GitBox <gi...@apache.org>.
rgoers commented on pull request #419:
URL: https://github.com/apache/logging-log4j2/pull/419#issuecomment-692148308
Yes, if there are new options then the Layout page should be updated accordingly. You should provide at least one example of using the options.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org