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

Adds RDoc summary of Hash methods #4045

Merged
merged 3 commits into from
Jan 10, 2021
Merged

Adds RDoc summary of Hash methods #4045

merged 3 commits into from
Jan 10, 2021

Conversation

BurdetteLamar
Copy link
Member

Adds a linked summary of methods by category.

@BurdetteLamar BurdetteLamar added the Documentation Improvements to documentation. label Jan 9, 2021
@BurdetteLamar BurdetteLamar changed the title Adds RDoc summary of methods Adds RDoc summary of Hash methods Jan 9, 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 pretty good, thanks for working on this! Just a few recommendations for improvements.

hash.c Outdated Show resolved Hide resolved
hash.c Outdated Show resolved Hide resolved
hash.c Outdated Show resolved Hide resolved
*
* ==== Methods for Fetching
*
* #[]:: Returns the value associated with a given key.
Copy link
Contributor

Choose a reason for hiding this comment

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

Mention it returns the default value if the key isn't present, similar with values_at.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd like to keep these 1-liners simple, and not indicate exceptional behavior.

* #[]:: Returns the value associated with a given key.
* #assoc:: Returns a 2-element array containing a given key and its value.
* #dig:: Returns the object in nested objects that is specified by a given key and identifiers.
* #fetch:: Returns the value for a given key.
Copy link
Contributor

Choose a reason for hiding this comment

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

Mention it raises a KeyError if the key is not present, similar with fetch_values.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd like to keep these 1-liners simple, and not indicate exceptional behavior.

hash.c Outdated Show resolved Hide resolved
hash.c Outdated Show resolved Hide resolved
hash.c Outdated
* #filter!:: Removes entries selected by a given block.
* #keep_if:: Removes entries selected by a given block.
* #reject!:: Removes entries selected by a given block.
* #select!:: Removes entries selected by a given block.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use different text for methods that aren't aliases. For #filter, #keep_if, #select, I would use "Removes entries not selected by a given block" or "Only keep entries selected by a given block".

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 think, but please re-review.

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.

Most changes look good. Still a couple changes that I think should be made.

hash.c Outdated
* #keep_if:: Keep only those entries selected by a given block.
* #reject!:: Removes entries selected by a given block.
* #shift:: Removes and returns the first entry.
* #slice:: Returns a hash containing the entries for given keys.
Copy link
Contributor

Choose a reason for hiding this comment

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

#slice returns a copy of self, it doesn't modify self, so it should be moved to the section below.

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.

* #compact:: Returns a copy of +self+ with all +nil+-valued entries removed.
* #except:: Returns a copy of +self+ with entries removed for specified keys.
* #filter, #select:: Returns a copy of +self+ with entries removed as specified by a given block.
* #reject:: Returns a copy of +self+ with entries removed as specified by a given block.
Copy link
Contributor

Choose a reason for hiding this comment

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

#filter/#select and #reject have the opposite behavior, you need to use different text for them. You could change removed to kept (or removed except) for #filter/#select to fix this.

Copy link
Member Author

Choose a reason for hiding this comment

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

#filter/#select now similar to #filter!/#select! above:

  • #filter, #select:: Returns a copy of +self+ with only those entries selected by a given block.

@BurdetteLamar BurdetteLamar merged commit 0e015f9 into ruby:master Jan 10, 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