Skip to content
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

Refactor/logging #495

Merged
merged 31 commits into from Apr 24, 2024
Merged

Refactor/logging #495

merged 31 commits into from Apr 24, 2024

Conversation

dom-apuliasoft
Copy link
Collaborator

@dom-apuliasoft dom-apuliasoft commented Apr 16, 2024

Description

Complete remake of the logging system

Checklist:

  • There are tests regarding this feature
  • The code follows the Kotlin conventions (run ./gradlew ktlintCheck)
  • The code passes all tests (run ./gradlew check)
  • There is a specific documentation in the docs directory

@lanarimarco lanarimarco self-assigned this Apr 19, 2024
Copy link
Collaborator

@lanarimarco lanarimarco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all, great work!!!

Please update logging.md document. We have a new channel, the log format has changed, and so on.

Add Kotlin docs for all new classes/interfaces defined.

In the PR comment, provide an overview of the relationships among the new classes in order to help the reader understand what happens under the hood when Jariko logs something.

Next time, change the code format only in cases of objectively incorrect formatting. For instance, I see that you prefer placing statements on multiple lines, whereas I prefer placing them on a single line unless the line exceeds 80 characters. You can use your preferred code format for the new code, but changing the code format for code not affected by your changes makes it more difficult for me to review the code.

@lanarimarco lanarimarco reopened this Apr 22, 2024
Copy link
Collaborator

@lanarimarco lanarimarco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add Kotlin docs for all new classes/interfaces defined.

It misses

In the PR comment, provide an overview of the relationships among the new classes in order to help the reader understand what happens under the hood when Jariko logs something.

It misses

And other stuff in logging.md

docs/logging.md Outdated Show resolved Hide resolved
docs/logging.md Show resolved Hide resolved
docs/logging.md Outdated Show resolved Hide resolved
docs/logging.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@lanarimarco lanarimarco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

@lanarimarco lanarimarco merged commit a3f41b9 into develop Apr 24, 2024
1 check passed
@lanarimarco lanarimarco deleted the refactor/logging branch April 24, 2024 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants