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

Added grow_fn and retain to Vec #13274

Closed
wants to merge 1 commit into from
Closed

Added grow_fn and retain to Vec #13274

wants to merge 1 commit into from

Conversation

pongad
Copy link
Contributor

@pongad pongad commented Apr 3, 2014

Fixes #13249

@huonw
Copy link
Member

huonw commented Apr 3, 2014

These need some tests.

@pongad
Copy link
Contributor Author

pongad commented Apr 3, 2014

My uninformed opinion is that the two functions are pretty simple and all the tests they need are in the examples. Should I add more? What kind of tests?

/// ```rust
/// let mut vec = vec!(1u,2,3);
/// vec.grow_fn(3, |i| i);
/// assert_eq!(vec, vec!(1, 2, 3, 1, 2, 3));
Copy link
Member

Choose a reason for hiding this comment

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

The documentation claims that it starts from 0, but this example claims otherwise.

I would recommend being sure to run the doc tests, they may find bugs!

@alexcrichton
Copy link
Member

I would also like to see some unit tests for these functions, in addition to the doc examples

@pongad
Copy link
Contributor Author

pongad commented Apr 3, 2014

I'm not sure why the Examples did not get tested. Added tests at the end of the module, I made sure those run.

@pongad
Copy link
Contributor Author

pongad commented Apr 4, 2014

Rebased

@bors bors closed this in 4cf8d8c Apr 5, 2014
@pongad pongad deleted the vec_add_grow branch April 5, 2014 00:28
Jarcho pushed a commit to Jarcho/rust that referenced this pull request Aug 24, 2024
…Frednet

Fix code snippet in from_str_radix_10 docs

`<expression>` was being treated as an opening HTML tag

changelog: none
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.

Vec should retain in-place methods
3 participants