Skip to content

Conversation

AlisdairO
Copy link
Contributor

As per topic :-).

Part of #24407.
r? @Manishearth

Copy link
Member

Choose a reason for hiding this comment

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

It's legal to import fns from crates

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe "It's not legal to directly import methods"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apologies, that was meant to read trait! I like tamird's wording better though.

Copy link
Member

Choose a reason for hiding this comment

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

agreed.

make sure you make the methods vs functions thing clear, for many people those are the same thing.

Copy link
Member

Choose a reason for hiding this comment

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

s/function/method?

@AlisdairO
Copy link
Contributor Author

doh, sorry!

Change made - happy to squash it if it looks good now.

@Manishearth
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jul 19, 2015

📌 Commit 4ea02b7 has been approved by Manishearth

@bors
Copy link
Collaborator

bors commented Jul 19, 2015

⌛ Testing commit 4ea02b7 with merge 2b7a7dc...

@bors
Copy link
Collaborator

bors commented Jul 19, 2015

💔 Test failed - auto-linux-32-opt

@Manishearth
Copy link
Member

@bors retry

Copy link
Contributor

Choose a reason for hiding this comment

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

why "in general"? is there an exception? this feels like the diagnostic is not sure why it's firing. Let's not ship slightly ambiguous language like this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No exceptions as far as I'm aware. Since this is mid-commit I can always make another PR to fix it up.

@bors
Copy link
Collaborator

bors commented Jul 19, 2015

⌛ Testing commit 4ea02b7 with merge cb87ea8...

bors added a commit that referenced this pull request Jul 19, 2015
@bors bors merged commit 4ea02b7 into rust-lang:master Jul 19, 2015
@AlisdairO AlisdairO deleted the diagnostics253 branch July 20, 2015 10:49
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.

5 participants