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

Fix Hash's initial_capacity documentation #8569

Merged
merged 1 commit into from Jan 7, 2020

Conversation

wooster0
Copy link
Contributor

@wooster0 wooster0 commented Dec 8, 2019

#8017 changed the default initial_capacity to 8.

@bew
Copy link
Contributor

bew commented Dec 8, 2019

Just an idea, since this is a rather arbitrary value afaik, we could have it in a constant INITIAL_CAPACITY, and in the documentation refer to it using a macro {{ INITIAL_CAPACITY }} to avoid these kind of desynchronization between the code and the docs?

@wooster0
Copy link
Contributor Author

wooster0 commented Dec 8, 2019

That doesn't sound too bad but I'm pretty sure that would require wrapping the entire constructors with {% begin %} and {% end %} and then that could become pretty ugly when there's also additional indentation in the {% begin %} and {% end %}. I'm not sure it's worth the complexity and I doubt the initial capacity is changing that often.

@straight-shoota
Copy link
Member

The default capacity actually isn't really important for users. It's an implementation detail. It's worth mentioning it, but not exposing it as a constant.

@bew
Copy link
Contributor

bew commented Dec 9, 2019

We could use a private constant to not expose this, but you're right, it's probably not worth the burden

@straight-shoota straight-shoota merged commit d8f2d1c into crystal-lang:master Jan 7, 2020
@straight-shoota straight-shoota added this to the 0.33.0 milestone Jan 7, 2020
@straight-shoota
Copy link
Member

Thank you @r00ster91 ❤️

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

Successfully merging this pull request may close these issues.

None yet

4 participants