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

Add documentation to Array#pop #3675

Merged
merged 1 commit into from Dec 13, 2016

Conversation

dylandrop
Copy link
Contributor

Adds documentation to two undocumented variants of Array#pop.

# ```
# a = [1]
# a.pop { "Testing" } #=> 1
# a.pop { "Testing" } #=> "Testing"
def pop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this method even meant to be publicly exposed? It sure is an odd duck in terms of behavior. If it's meant to be private, perhaps it's best if it's hidden from the API docs altogether.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it's public. Maybe you'd like to pop a value or use another default value instead of nil

@@ -1201,6 +1201,13 @@ class Array(T)
pop { raise IndexError.new }
end

# Removes the last value from `self`.
# If the array is of size 0, the given block is called.
Copy link
Member

Choose a reason for hiding this comment

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

What about "If the array is empty"?

@dylandrop
Copy link
Contributor Author

@asterite Updated.

@asterite
Copy link
Member

Thanks!

@asterite asterite merged commit 0f4bdc1 into crystal-lang:master Dec 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants