Skip to content

Conversation

@mdinger
Copy link
Contributor

@mdinger mdinger commented Nov 8, 2014

I had slight confusion when using this as a reference and was told it was imprecise. Most of the rewording was suggested by @huonw.

cc @steveklabnik

@aturon
Copy link
Contributor

aturon commented Nov 8, 2014

Thanks for the PR! This looks like an improvement to me, but I want to give @steveklabnik a chance to weigh in.

@mdinger
Copy link
Contributor Author

mdinger commented Nov 8, 2014

Specifically, with coerce as before, I interpreted it to mean that the underlying String was modified by as_slice() which was puzzling.

@steveklabnik
Copy link
Contributor

We haven't made a formal decision to use this 'view' terminology, but I do like it. I am neutral to mildly positive about this, so if @aturon likes it, r=me

@mdinger
Copy link
Contributor Author

mdinger commented Nov 8, 2014

I don't get how r=me is supposed to work. I could just r=me at aturon or steveklabnik or alexcrichton and bors would automatically merge without their explicit approval (I know how it's supposed to be written but am not trying to write it properly here)? Isn't that just bypassing the review process?

@aturon
Copy link
Contributor

aturon commented Nov 9, 2014

@Mdiner It's just an informal way of telling other reviewers that you're personally happy with the PR. Someone with reviewing rights still has to sign off.

(bors will only pay attention to r+ and r=somereviewer comments on the commit from people with reviewing rights.)

@aturon
Copy link
Contributor

aturon commented Nov 9, 2014

@steveklabnik Yeah, we should think more broadly about this terminology -- might be worth raising at the next weekly meeting. Want me to add to the agenda?

In any case, I'm happy to land this for now.

@mdinger
Copy link
Contributor Author

mdinger commented Nov 9, 2014

Okay. Thanks for clarification

@steveklabnik
Copy link
Contributor

@aturon sounds great, yeah. I'll come prepared :)

bors added a commit that referenced this pull request Nov 9, 2014
I had slight confusion when using this as a reference and was told it was imprecise. Most of the rewording was suggested by @huonw.

cc @steveklabnik
@bors bors closed this Nov 9, 2014
@bors bors merged commit c39e9f3 into rust-lang:master Nov 9, 2014
@mdinger mdinger deleted the str_coerce branch January 2, 2015 20:51
lnicola pushed a commit to lnicola/rust that referenced this pull request Jan 7, 2025
…salsa-cancellation-error

internal: wrap `salsa::Cycle`
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