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

Rdoc enhancements for psych.rb #468

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

BurdetteLamar
Copy link
Member

This PR concerns primarily the high-level interface:

  • Added introductory material, with links to the spec and Ruby cookbook at yaml.org.
  • Modified high-level API discussion, with new examples and links to the relevant methods.
  • Added sections on options, which the methods can link to.
  • Enhanced doc for load and dump methods:
    • load
    • load_file
    • load_stream
    • dump
    • dump_stream

This PR is larger than I would have liked, but the high-level API concepts are intertwined, and the their doc needs to be modified all at once.

@BurdetteLamar
Copy link
Member Author

@hsbt, I've requested a review from you because you're the frequent contributor in recent history. Let me know if you'd like me to request someone else.

@BurdetteLamar
Copy link
Member Author

@hsbt, should I ask someone else to review?

@BurdetteLamar
Copy link
Member Author

@hsbt, I think this is ready for review.

Copy link
Contributor

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

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

I like this change, in that it strengthens the "general section" which provides a complete narrative, for the learner/reader.

lib/psych.rb Outdated Show resolved Hide resolved
lib/psych.rb Outdated Show resolved Hide resolved
BurdetteLamar and others added 2 commits May 10, 2021 09:27
Co-authored-by: Olle Jonsson <[email protected]>
Co-authored-by: Olle Jonsson <[email protected]>
Copy link
Contributor

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

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

Looks ✨ good ✨ to me!

@BurdetteLamar
Copy link
Member Author

@hsbt, @olleolleolle has reviewed and approved this PR. Ok to merge? I'm reluctant without hearing from you.

@BurdetteLamar
Copy link
Member Author

@tenderlove, would appreciate a blessing from you or @hsbt before merging this.

@hsbt
Copy link
Member

hsbt commented May 12, 2021

@BurdetteLamar You ignored our feedback at ruby/ruby#3506 (comment)

Because I couldn't/will not approve your pull-request.

@BurdetteLamar
Copy link
Member Author

@tenderlove, you are a maintainer here? Can you (or someone) review this?

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.

3 participants