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

[DOC] Enhanced RDoc for IO #5307

Merged
merged 3 commits into from Dec 20, 2021
Merged

[DOC] Enhanced RDoc for IO #5307

merged 3 commits into from Dec 20, 2021

Conversation

BurdetteLamar
Copy link
Member

Treated:

  • #sync
  • #sync=
  • #fsync
  • #fdatasync
  • #fileno
  • #pid
  • #inspect
  • #to_io

@BurdetteLamar BurdetteLamar added the Documentation Improvements to documentation. label Dec 20, 2021
Copy link
Contributor

@jeremyevans jeremyevans left a comment

Choose a reason for hiding this comment

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

Looks mostly good. Only a few minor changes requested, please see inline comments.

io.c Outdated
* system and is not buffered by Ruby internally. See also
* IO#fsync.
* Returns the value of the most recent call to IO#sync=,
* or +false+ if the method has not been called; see IO#sync=:
Copy link
Contributor

Choose a reason for hiding this comment

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

The previous language was more accurate. There are cases where the sync mode is set without IO#sync= (such as on the write pipe returned by IO.pipe). I would continue to state that this returns the sync mode, even if you only define the sync mode in IO#sync=

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

io.c Outdated
@@ -2630,22 +2650,23 @@ rb_io_descriptor(VALUE io)

/*
* call-seq:
* ios.pid -> integer
* pid -> integer
Copy link
Contributor

Choose a reason for hiding this comment

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

integer or nil

Somewhere in the description, we should describe that this returns nil for instances not created by IO.popen.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. I saw the nil in the previous doc, but could not figure out when it occurs.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the call-seq should still be updated to say integer or nil.

Copy link
Contributor

@jeremyevans jeremyevans left a comment

Choose a reason for hiding this comment

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

Please change the call-seq for #pid before committing. After that, OK to commit without further review.

io.c Outdated
@@ -2630,22 +2650,23 @@ rb_io_descriptor(VALUE io)

/*
* call-seq:
* ios.pid -> integer
* pid -> integer
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the call-seq should still be updated to say integer or nil.

@BurdetteLamar BurdetteLamar merged commit 6ad8cf7 into ruby:master Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Improvements to documentation.
2 participants