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 ::ln_s #72

Merged
merged 4 commits into from May 23, 2022
Merged

[DOC] Enhanced RDoc for ::ln_s #72

merged 4 commits into from May 23, 2022

Conversation

BurdetteLamar
Copy link
Member

No description provided.

@BurdetteLamar
Copy link
Member Author

@jeremyevans, I'm still feeling my way here:

  • There's has a separate fileset (with its own n) for each argset; this means that:
    • The reader doesn't have any carryover to remember.
    • Everything is still valid when we get to verbose: true at the bottom.
  • Under verbose: true, should the five commands be interleaved with the five method calls?
  • Alternatively, should each verbose call and its output be put with the corresponding example code above?

As our strategy evolves, I may need to revise some of the previous work here.

lib/fileutils.rb Outdated
#
# FileUtils.ln_s('src2.txt', 'dest2.txt') # Raises Errno::EEXIST.
#
# When +src+ and +dest+ are both paths to directories,
Copy link
Contributor

Choose a reason for hiding this comment

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

This behavior when dest is a directory is the same when src is a file or a directory. I don't think we need a separate example block for it, maybe just update the above text to mention that it applies both when src is a file and when it is a directory.

Copy link
Member Author

Choose a reason for hiding this comment

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

Cannot add this to the relevant list item above b/c the covering text says "When +src+ is the path to an existing file:".

Copy link
Contributor

Choose a reason for hiding this comment

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

Then consider reorganizing the sections to avoid the repetition.

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. One section now, with examples with src as both file and directory.

lib/fileutils.rb Outdated Show resolved Hide resolved
lib/fileutils.rb Outdated Show resolved Hide resolved
@BurdetteLamar
Copy link
Member Author

@jeremyevans, there's no re-review thingy, but this is ready for you to check.

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 pick an example group to remove (see comment), then this is OK to merge without further review.

lib/fileutils.rb Outdated
# When +dest+ is the path to a directory,
# creates a symbolic link at <tt>dest/src</tt> pointing to +src+:
#
# # File src:
Copy link
Contributor

Choose a reason for hiding this comment

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

Same behavior for both file/directory src cases, so let's just pick one and use it, there is no benefit for keeping both examples.

@BurdetteLamar BurdetteLamar merged commit db612c5 into ruby:master May 23, 2022
@BurdetteLamar
Copy link
Member Author

@jeremyevans, I've re-confirmed the examples, including the verbose output, b/c of the renumbering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants