-
-
Notifications
You must be signed in to change notification settings - Fork 51
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
Ruby: log via Asciidoctor logger to provide the user with a file name #199
Conversation
691b3a6
to
1134215
Compare
rebased, not sure why JS test failed in Windows - unstable test? |
Probably, we might need to adjust the timeout... |
I think we need to merge #171 first (or at least we will need to update #171 if we decide to merge this change first). If you have time to review #171 that would be great. I did these changes to allow a better integration with GitLab. I guess the Intellij IDEA plugin would also benefit from these changes but I didn't check carefully. |
I merged #171, we need to rebase this pull request. |
1134215
to
e678f03
Compare
@ahus1 Sorry I lost track but I think it's ready to go, right? |
e678f03
to
c18a087
Compare
@Mogztter - I've finally update the code, and updated also the other places where a logger was used. To be able to use I see that the code allows to pass a logger as a parameter. I had a look around and saw no code fragment using it. I wonder: What was the intention to not use the I suppose it is now ready for review. |
Sounds reasonable.
Yes, but also for integration. For instance, GitLab is using its own logger so it's important to be able to provide a different logger.
OK, thanks, I will take a look. |
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.
LGTM 👍🏻
I will wait a bit if someone else want to review before merging.
… and line number in the log, plus redirection of log content in IDEs.
I increased the "Metrics/AbcSize" as I didn't know a good refactoring for this method to stay below that limit. I'd appreciate a hint or help here.
Something I found: the log line will point at...
I started to pass down this information to KrokiProcessor.process, but then Rubocop complained about too many parameters. Then I settled with "leave it as it is" :-/ ... see this snippet on how to adjust line numbers for logging.