skip(Clone)

[ModProg]

fixes #104

[daxpedda]

Just hypothetically, what will we do when we get to a breaking change? Will we ever want derive_where(skip) to make Clone just call default() instead?

I'm thinking that letting Clone call default() under the term skip is not ideal and would clash with the general usage of derive_where(skip). I propose instead a new attribute: derive_where(clone_default) or derive_where(clone(default)). WDYT?

Will make a review of the code properly after figuring this out, because it would change the code entirely probably.

[ModProg]

I think I'm getting to a point there I'm questioning the global skip as a feature. I think it makes sense to have skips for EqHash etc. but just skipping for all traits that currently support the feature seems strange to me now

[daxpedda]

I think I'm getting to a point there I'm questioning the global skip as a feature. I think it makes sense to have skips for EqHash etc. but just skipping for all traits that currently support the feature seems strange to me now

Didn't cross my mind at all, but seems quite reasonable to me. In hindsight, a global skip almost looks like a footgun.

But unrelated, right? Or did you mean to keep skip(Clone) instead of a dedicated attribute considering you want to remove the global skip in the future?

[ModProg]

I feel like skip(Clone) is in line with other things that don't explicitly specify that Default will be used, when "removing" a value e.g. https://doc.rust-lang.org/std/mem/fn.take.html

[daxpedda]

Hm, skip to me implies that nothing is done, like with all the other traits skip supports right now. Clone definitely seems like a departure to me in this regard. While default() can imply something like "empty", as with std::mem::take(), it seems still different to me then "nothing" as "not used" or "skipped".

I will go ahead and review nevertheless, not a big issue really.

[ModProg]

I see your point, but thought of skip more as not doing the specific thing in this case cloning, but still doing what is required to allow the skipping, in this case defaulting.

[daxpedda]

Looks great!
Just needs an update of the README.

[ModProg]

@daxpedda we need to mention that skip without group does not include clone

[ModProg]

maybe referencing #111

[daxpedda]

I specifically did not include Clone in the list for blanket skipping, but only in the list for group skips.
Doesn't that cover it?

[ModProg]

True, should be fine, especially with the aforementioned planed deprecation of the "skip all" feature.