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

Rename functions in the CloneableVector trait #15725

Closed
wants to merge 1 commit into from
Closed

Rename functions in the CloneableVector trait #15725

wants to merge 1 commit into from

Conversation

aochagavia
Copy link
Contributor

  • Deprecated to_owned in favor of to_vec
  • Deprecated into_owned in favor of into_vec

[breaking-change]

* Deprecated `to_owned` in favor of `to_vec`
* Deprecated `into_owned` in favor of `into_vec`

[breaking-change]
@aochagavia
Copy link
Contributor Author

The travis build errored (timeout) so I have amended the commit without changing anything and pushed again. Hopefully it will go right this time.

@aochagavia
Copy link
Contributor Author

cc @aturon. I suppose you will find this interesting, since you are working on the libraries.

@@ -1273,9 +1274,9 @@ impl<'l> Visitor<DxrVisitorEnv> for DxrVisitor<'l> {
// process collected paths
for &(id, ref p, ref immut, ref_kind) in self.collected_paths.iter() {
let value = if *immut {
self.span.snippet(p.span).into_owned()
self.span.snippet(p.span).into_string()
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, the snippet method seems to produce a String already, but maybe I'm missing something?

@aturon
Copy link
Member

aturon commented Jul 17, 2014

For the record, this patch also seems to be catching some into_owned cases that were missed in the into_string migration, which is fine. This looks good to me, though I'm surprised there weren't more uses of the _owned functions. It might be worth trying to remove them outright, fixing any errors, and then putting back the deprecated versions just to make sure all uses have been caught.

Thanks for doing this work!

@aturon
Copy link
Member

aturon commented Jul 17, 2014

I'm not sure whether the [breaking-change] tag is needed here, since the old methods are available in deprecated form. @alexcrichton, thoughts?

@alexcrichton
Copy link
Member

I view this as inciting a breaking change because eventually we will be removing the older functions, so we should act as if they're removed right now for rust code.

In a procedural sort of sense, I highly doubt that whoever removes these deprecated functions will remember to annotate the change with a [breaking-change], and furthermore there's more precise information in this commit message about the breaking change than there would be in a remove-all-deprecated-things commit.

bors added a commit that referenced this pull request Jul 18, 2014
* Deprecated `to_owned` in favor of `to_vec`
* Deprecated `into_owned` in favor of `into_vec`

[breaking-change]
@bors bors closed this Jul 18, 2014
@aochagavia aochagavia deleted the vec branch July 18, 2014 06:56
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 13, 2023
internal: fix automatic rustc/rustdoc lint generation

Missed in rust-lang#15680: the output of `-Whelp` changed since the last run so it generated some bad rustdoc lints entries.
Also preemptively fix a `-Whelp` breakage that might get merged in rust-lang#116412
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.

4 participants