feat!: replace peel_to_x by peel_to_x_in_place#2146
feat!: replace peel_to_x by peel_to_x_in_place#2146cruessler wants to merge 1 commit intoGitoxideLabs:mainfrom
Conversation
This commit adds the suffix `in_place` so some methods that mutate a `Reference` in place, in order to make that effect more obvious.
|
Thanks for kicking this off! When looking at this I noticed that…
What we are really trying to improve here is what happens in the relatively rare case when When looking at the method descriptions, often they don't explicitly point out what it means that they are Does that sounds sane to you? I also thought about internally copying the ref each time, but that would be going against I also also thought about having a sibling method that does the copy internally, but then I thought one could rather Last but not least, maybe there is another verb, i.e. not peel, that would justify working on a copy? Then there could be a sibling method. And last but not least, having these functions mutable is a good thing as a fully peeled ref won't incur more work when other versions of these methods are called on them, so I think it's good. |
|
I fully agree and have opened a follow-up PR: #2163. |
|
Thanks so much for the follow-up! To me it also looks like it supersedes this one, so I am closing it. Please feel free to reopen if this one should be used for something else. |
This PR adds the suffix
in_placeso some methods that mutate aReferencein place, in order to make that effect more obvious. It is based on this suggestion: #2142 (comment).This PR intentionally is very small, for two reasons: 1) in order to make it easier to review, 2) because I’m not sure how many methods to rename and where to stop. There is a number of methods that probably also should be renamed, such as
peel_to_kind,peel_to_kind_packed, and a couple offollow_to_xmethods. I haven’t done that yet because they sit at a lower level and we probably would have to change calls in higher-level methods to use the renamed methods. So, for example,peel_to_blob_in_placecurrently can still callpeel_to_kind, but would have to callpeel_to_kind_in_placeif we wanted to be thorough.What do you think?