-
Notifications
You must be signed in to change notification settings - Fork 59
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
[DOC] Enhanced RDoc for Logger #77
Conversation
lib/logger.rb
Outdated
# Reopen a log device. | ||
# Sets the logger's output stream: | ||
# | ||
# - If +logdev+ is +nil+, reopens the already-established output stream. |
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.
I'm a bit confused as to what "already-established" means. Do you think "current" is a better word to use?
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.
Fixed.
lib/logger.rb
Outdated
# | ||
def reopen(logdev = nil) | ||
@logdev&.reopen(logdev) | ||
self | ||
end | ||
|
||
# Creates a log entry (which may or may not be written to the log). |
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.
I think we should be clear that it "may or may not be written to the log" because of the log level (and not because it does sampling or intermittently fails or some other reason).
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.
Fixed.
lib/logger.rb
Outdated
# When you call instance method #add (or its alias #log), | ||
# an entry may (or may not) be written to the log; | ||
# When you call any of these methods, | ||
# the entry may (or may not) be written to the log; |
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.
Ditto on clarifying the "may or may not".
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.
Fixed.
Co-authored-by: Peter Zhu <peter@peterzhu.ca>
Co-authored-by: Peter Zhu <peter@peterzhu.ca>
Co-authored-by: Peter Zhu <peter@peterzhu.ca>
Co-authored-by: Peter Zhu <peter@peterzhu.ca>
Co-authored-by: Peter Zhu <peter@peterzhu.ca>
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.
Looks good.
Could you use the "Squash and merge" button when merging to squash all of the commits into one?
Enhanced RDoc for Logger ruby/logger@c601ed0370 Co-authored-by: Peter Zhu <peter@peterzhu.ca>
Enhanced RDoc for Logger ruby/logger@c601ed0370 Co-authored-by: Peter Zhu <peter@peterzhu.ca>
Enhanced RDoc for Logger ruby/logger@c601ed0370 Co-authored-by: Peter Zhu <peter@peterzhu.ca>
This covers the methods and accessors, along with further changes and reorgs in the class doc itself.