Fix docs on ToArrayError
#169
Conversation
The docs are slightly incorrect. Make them match those on the `ToBytesError` type. Noticed while testing the upcoming alpha release.
tcharding
added
the
trivial
| @@ -207,7 +207,7 @@ impl From<InvalidLengthError> for HexToArrayError { | |||
| fn from(e: InvalidLengthError) -> Self { Self(e.into()) } | |||
| } | |||
|
|
|||
| /// Hex decoding error while parsing a byte array. | |||
| /// Hex decoding error while parsing to an array of bytes. | |||
| #[derive(Debug, Clone, PartialEq, Eq)] | |||
| pub enum ToArrayError { | |||
There was a problem hiding this comment.
Whaat??? Why are we calling these error types To*Error? Why not Parse*Error or Decode*Error?
Oh, I think I know. Wasn't it previously called HexTo*Error? So the intended use is hex::To*Error?
There was a problem hiding this comment.
ooph, I did not see this discussion until I posted elsewhere and now I don't know where that was. Goodness me. Anyway, I attempted to explain how these error types came to exist someplace - you'll see it in your notifications hopefully.
| @@ -207,7 +207,7 @@ impl From<InvalidLengthError> for HexToArrayError { | |||
| fn from(e: InvalidLengthError) -> Self { Self(e.into()) } | |||
| } | |||
|
|
|||
| /// Hex decoding error while parsing a byte array. | |||
| /// Hex decoding error while parsing to an array of bytes. | |||
There was a problem hiding this comment.
I like the previous wording better so if you want to make them consistent, I'd rather change the other one. After some thinking, I don't like either. I think "decoding into an array of bytes" is better. But also I'm not sure about using the word "parsing" vs "decoding". I think "decoding" fits better in the context of this crate. (Of course, if you have some hex-encoded object like PublicKey then the word "parsing" makes more sense.)
|
On wording. "To decode X <no to/into/from here>" - I believe X refers to the input (hex in this case). However "to parse X" - X refers to the output but the term "parse" is used for objects not encoding transformations. "To decode X from Y" Y is input, X is output. So we want the word "decode" and we want to be explicit about output type, so we need to say "decode hex into Also for some reason into feels better than to here but I have no idea why. It's just intuition. |
|
Agreed with your comments in general. I also don't mind renaming the error types since the pain will be fairly limited (only people wrapping the errors will be hit by them). Though for the same reason I really don't care about them. Also agreed that we should use "decode" and "encode" rather than "parse" and "serialize". Hex to bytes is not transforming the data in any way, just changing the encoding. But what should we do about this PR? Change it so that it renames the types? Or can we accept the doc changes as is? |
|
My suggestion is, to change "parsing to" to "decoding into" at the place this PR modifies and also at the other place that supposedly should be the same to fulfill the goal of the PR. |
The docs are slightly incorrect. Make them match those on the
ToBytesErrortype.Noticed while testing the upcoming alpha release.