Add support for logging with multiple tags #213

kahgoh · 2021-06-16T13:53:09Z

Description

Linked issue: #201

This change allows a single TaggedLogger to log to multiple tags, instead of having to create individual logger for each tag.

Definition of Done

I read contributing.md
There are no TODOs left in the code
Code style follows the tinylog standard
All classes and methods have Javadoc
Changes are covered by JUnit tests including edge cases, errors, and exception handling
Maven build works including compiling, tests, and checks (mvn verify)
Changes are committed by a verified email address that is assigned to the GitHub account (https://github.com/settings/emails)

Documentation

Additions or amendments for the public documentation:

Tags will need to be updated with the addition of the new tags method.

Agreements

I agree that my changes will be published under the terms of the Apache License 2.0 (mandatory)
I agree that my GitHub user name will be published in the release notes (optional)

pmwmedia · 2021-06-29T12:04:58Z

What is the current status? When can I do the review?

kahgoh · 2021-06-30T14:15:09Z

Sorry, its taking me a while. I'm still working updating the Scala API side.

pmwmedia · 2021-07-01T21:50:00Z

@kahgoh Thanks for the status update :)

kahgoh · 2021-07-05T13:16:41Z

I've pushed in the Scala API changes and think its ready for review now. I'll come back to add something for the documentation later when I get the chance.

codecov · 2021-07-05T14:10:02Z

Codecov Report

Merging #213 (1b84437) into v2.4 (e1019aa) will increase coverage by 0.05%.
The diff coverage is 94.05%.

@@             Coverage Diff              @@
##               v2.4     #213      +/-   ##
============================================
+ Coverage     94.33%   94.39%   +0.05%     
- Complexity     2463     2638     +175     
============================================
  Files           128      131       +3     
  Lines          4699     5047     +348     
  Branches        529      655     +126     
============================================
+ Hits           4433     4764     +331     
- Misses          146      156      +10     
- Partials        120      127       +7

Impacted Files	Coverage Δ
...g/tinylog/writers/AbstractFormatPatternWriter.java	`100.00% <ø> (+2.22%)`	⬆️
...cala/src/main/scala/org/tinylog/scala/Logger.scala	`88.23% <83.33%> (-3.44%)`	⬇️
.../src/main/java/org/tinylog/writers/JsonWriter.java	`88.57% <88.57%> (ø)`
...src/main/kotlin/org/tinylog/kotlin/TaggedLogger.kt	`95.42% <93.38%> (+0.64%)`	⬆️
...og-api/src/main/java/org/tinylog/TaggedLogger.java	`96.55% <94.54%> (-0.19%)`	⬇️
...a/org/tinylog/writers/AbstractFileBasedWriter.java	`97.43% <97.43%> (ø)`
...org/tinylog/slf4j/TinylogSlf4jServiceProvider.java	`100.00% <100.00%> (ø)`
...otlin/src/main/kotlin/org/tinylog/kotlin/Logger.kt	`96.00% <100.00%> (+0.83%)`	⬆️
...rc/main/scala/org/tinylog/scala/TaggedLogger.scala	`100.00% <100.00%> (ø)`
tinylog-api/src/main/java/org/tinylog/Logger.java	`96.51% <100.00%> (+0.26%)`	⬆️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5679944...1b84437. Read the comment docs.

pmwmedia · 2021-07-05T14:34:10Z

Thank you very much for your commit :) I'm going to review it. Could you fix one compile error in the mean time?

/home/runner/work/tinylog/tinylog/tinylog-api/src/main/java/org/tinylog/Logger.java:[92,47] diamond operator is not supported in -source 1.6

For Android support, the source code still needs to be Java 6 compatible.

tinylog-api-kotlin/src/main/kotlin/org/tinylog/kotlin/Logger.kt

tinylog-api-kotlin/src/main/kotlin/org/tinylog/kotlin/TaggedLogger.kt

tinylog-api-scala/src/main/scala/org/tinylog/scala/Logger.scala

tinylog-api/src/main/java/org/tinylog/Logger.java

tinylog-api/src/main/java/org/tinylog/TaggedLogger.java

tinylog-api-kotlin/src/main/kotlin/org/tinylog/kotlin/TaggedLogger.kt

tinylog-api-kotlin/src/test/kotlin/org/tinylog/kotlin/LoggerTest.kt

pmwmedia · 2021-07-05T21:43:12Z

I have just finished my review. Your code looks very well! I have found just a few minors to change.

See issue #201.

kahgoh · 2021-07-08T13:43:07Z

Thanks for the feedback and review! I have just finished and pushed the changes.

pmwmedia · 2021-07-08T14:15:12Z

tinylog-api-kotlin/src/main/kotlin/org/tinylog/kotlin/TaggedLogger.kt

-class TaggedLogger internal constructor(private val tag: String?) {
+class TaggedLogger internal constructor(private val tags: Set<String?>) {
+
+	constructor(tag: String?) : this(setOf(tag))


Do we really still need this constructor? If yes, could you declare the constructor as internal? I want to prohibit that users create their own instance.

Nope, that constructor isn't needed anymore. I'll remove it 😉

pmwmedia · 2021-07-08T14:37:45Z

Thank you very much for your changes! I could found only one last minor.

Could to push potential further changes as new separate commits? It would make the review easier for me :)

I really like your pull request! You had to change a lot of code. However, your changes are easily comprehensible.

This simplifies the class and prevents users from initiating their own instances (they should be obtaining instances via the Logger).

kahgoh · 2021-07-09T13:52:03Z

I've just pushed the change to remove the internal constructor

pmwmedia · 2021-07-09T20:47:03Z

Thank you for your great work! I have merged your pull request with pleasure.

github-actions · 2022-10-09T12:16:00Z

This closed pull request has been locked automatically. However, please feel free to file an issue or create a new pull request.

kahgoh marked this pull request as ready for review July 5, 2021 13:16

pmwmedia requested changes Jul 5, 2021

View reviewed changes

Add ability to log to multiple tags

2d93679

See issue #201.

pmwmedia requested changes Jul 8, 2021

View reviewed changes

Remove single tag constructor from TaggedLogger.kt

1b84437

This simplifies the class and prevents users from initiating their own instances (they should be obtaining instances via the Logger).

pmwmedia approved these changes Jul 9, 2021

View reviewed changes

pmwmedia merged commit 26dc4aa into tinylog-org:v2.4 Jul 9, 2021

pmwmedia mentioned this pull request Jul 9, 2021

Logger with multiple tags #201

Closed

github-actions bot locked as resolved and limited conversation to collaborators Oct 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for logging with multiple tags #213

Add support for logging with multiple tags #213

kahgoh commented Jun 16, 2021 •

edited

pmwmedia commented Jun 29, 2021

kahgoh commented Jun 30, 2021 •

edited

pmwmedia commented Jul 1, 2021 •

edited

kahgoh commented Jul 5, 2021 •

edited

codecov bot commented Jul 5, 2021 •

edited

pmwmedia commented Jul 5, 2021

pmwmedia commented Jul 5, 2021

kahgoh commented Jul 8, 2021

pmwmedia Jul 8, 2021

kahgoh Jul 9, 2021

pmwmedia commented Jul 8, 2021

kahgoh commented Jul 9, 2021

pmwmedia commented Jul 9, 2021

github-actions bot commented Oct 9, 2022

Add support for logging with multiple tags #213

Add support for logging with multiple tags #213

Conversation

kahgoh commented Jun 16, 2021 • edited

Description

Definition of Done

Documentation

Agreements

pmwmedia commented Jun 29, 2021

kahgoh commented Jun 30, 2021 • edited

pmwmedia commented Jul 1, 2021 • edited

kahgoh commented Jul 5, 2021 • edited

codecov bot commented Jul 5, 2021 • edited

Codecov Report

pmwmedia commented Jul 5, 2021

pmwmedia commented Jul 5, 2021

kahgoh commented Jul 8, 2021

pmwmedia Jul 8, 2021

Choose a reason for hiding this comment

kahgoh Jul 9, 2021

Choose a reason for hiding this comment

pmwmedia commented Jul 8, 2021

kahgoh commented Jul 9, 2021

pmwmedia commented Jul 9, 2021

github-actions bot commented Oct 9, 2022

kahgoh commented Jun 16, 2021 •

edited

kahgoh commented Jun 30, 2021 •

edited

pmwmedia commented Jul 1, 2021 •

edited

kahgoh commented Jul 5, 2021 •

edited

codecov bot commented Jul 5, 2021 •

edited