Skip to content

Conversation

shahn
Copy link
Contributor

@shahn shahn commented Dec 27, 2016

This seems to be consistent with all the Extend implementations I found, and isn't documented anywhere else afaik.

@rust-highfive
Copy link
Contributor

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@shahn
Copy link
Contributor Author

shahn commented Dec 27, 2016

r? @steveklabnik

@rust-highfive rust-highfive assigned steveklabnik and unassigned aturon Dec 27, 2016
@steveklabnik
Copy link
Contributor

@rust-lang/libs is this something we want to guarantee here?

@sfackler
Copy link
Member

sfackler commented Jan 3, 2017

I would probably move this documentation to the implementations themselves - this wording may not make sense for a multimap or multiset, for example.

@pnkfelix
Copy link
Contributor

pnkfelix commented Jan 3, 2017

@sfackler one might argue the phrase "the entry is updated" is open-ended enough to cover multimaps/multisets. It is certainly more appropos than saying "the entry is replaced", at least to me.

(I assume that the critical detail the author desires is to convey that such entries are not ignored during the traversal. Is that open-ended enough to commit to?)

@shahn
Copy link
Contributor Author

shahn commented Jan 3, 2017

Yes indeed, that's what this is about. How about the following wording: the entry is updated or newly added if multiple elements with the same key are permitted."?

@shahn
Copy link
Contributor Author

shahn commented Jan 5, 2017

I have pondered a bit on the wording and provided an updated wording, is that better? I would really like to avoid making the Extend behaviour implementation documented, because that's a lot of places to look and a chance for confusing behaviour/inconsistent docs to creep in.

@steveklabnik
Copy link
Contributor

I like this wording, I think. @sfackler @pnkfelix ?

@sfackler
Copy link
Member

sfackler commented Jan 5, 2017

👍

@steveklabnik
Copy link
Contributor

@bors: r+ rollup

thanks!

@bors
Copy link
Collaborator

bors commented Jan 11, 2017

📌 Commit 74b2587 has been approved by steveklabnik

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jan 13, 2017
Clarify Extend behaviour wrt existing keys

This seems to be consistent with all the Extend implementations I found, and isn't documented anywhere else afaik.
bors added a commit that referenced this pull request Jan 13, 2017
Rollup of 10 pull requests

- Successful merges: #38362, #38636, #38877, #38946, #38965, #38986, #38994, #38995, #39024, #39027
- Failed merges:
@bors bors merged commit 74b2587 into rust-lang:master Jan 14, 2017
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.

7 participants