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

Reword explanations on the return type's lifetime #1782

Closed
wants to merge 1 commit into from

Conversation

Shiroy
Copy link

@Shiroy Shiroy commented Jan 22, 2019

The current explanation is telling that the return type's lifetime is at leat 'a.
It suggests that this reference outlives the &strs given in the input arguments.
This is not correct as this would creates invalid references.

The current explantion is telling that the return type's lifetime is at leat 'a.
It suggests that this reference outlives the `&str`s given in the input arguments.
This is not correct as this would create invalid references.
@mulkieran
Copy link
Contributor

Btw, the Travis failure is all about some code that doesn't compile in Chapter 14, i.e., it is spurious.

Copy link
Contributor

@mulkieran mulkieran left a comment

Choose a reason for hiding this comment

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

I believe that this is exactly the correction required.

@mulkieran
Copy link
Contributor

@Shiroy Could you do a rebase and a force push to jiggle Travis into action (and hopefully passing). I'm not a core Rust reviewer, but I would like this to go in, and I think it has a better chance if the CI is passing.

@mulkieran
Copy link
Contributor

@carols10cents This is an obviously correct bug fix, rather than a mere rewording for clarity.

@cuviper
Copy link
Member

cuviper commented Apr 8, 2019

I disagree -- #1875 (comment)

@carols10cents
Copy link
Member

See discussion on #1875; this is correct as-is. Thanks though!

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