Closed blyxyas closed 1 year ago
None of these implementations make sense.
I want to implement this for a custom type for my library, and also I want my library to use the Rust API Guidelines, that's why I made this PR. That's kinda rude tbh
I apologize for being short. PRs like this can get repetitive and frustrating, but I understand you were just trying to help so I'll give you a bit of context
First, PR review takes time. When you submit a PR for a new feature, you're not doing the maintainer a favor, you're asking for a favor; unless they requested the feature, you likely want it more than they do. This means the onus is on you to do as much work as possible up-front to avoid wasting their time.
The core issue with this PR is that it lacks any real motivation or forethought. A motivation both gives the reviewer context and forces you to think understand/think about the feature you're implementing.
In this specific case:
TempDir
gets dropped, the second will now refer to a non-existent directory. If you need this to be clonable, use an Rc
/Arc
.Display
isn't implemented for the same reason it isn't implemented on Path
(documented in Path::display()
).Eq
/Hash
are useless because, if you have multiple TempDir
structures referring to the same directory, you already have an issue (same as cloning). TempDir
s should be unique.you're not doing the maintainer a favor, you're asking for a favor
Oh I'm very sorry for taking time out of my day, thinking that adding some more traits could be helpful. Forking your repo, implementing the change and opening a Pull Request explaining the changes. Ignorant me!
I'm sorry! Thanks for the favor of saying "None of this implementations makes sense" and closing it without any further explanation until I point that out!
avoid wasting their time.
The change is made of a one-liner and a feature in Cargo.toml
. Omg such a waste of time!
Thanks for being so kind to let me ask for this favor! So that you could just be rude and close it instantly, without an explanation!
The Rust API Guidelines recommend deriving / implementing the following traits C-COMMON-TRAITS:
Debug
is already implemented andDisplay
andCopy
aren't derivable in this case. So this PR derives all remaining traits, and implementsDisplay
for TempDir.It also adds a "serde" feature (C-SERDE), if it's enabled, Serde's
Serialize
andDeserialize
are also derived.