Skip to content

Conversation

@ancane
Copy link
Contributor

@ancane ancane commented Dec 2, 2016

Hi. To me, having numerical keys is handy at times, and having to define custom format every time is cumbersome. So I'd like to suggest to put it here. Thanks.
This is my original work.

@ktoso
Copy link
Member

ktoso commented Dec 2, 2016

Refs #125

@lniskanen
Copy link

I had to merge this into an unmanaged jar, but hopefully this gets into release as well. Thanks for the fix.

@ancane
Copy link
Contributor Author

ancane commented Mar 15, 2017

np, thank you.

@eugeniyk
Copy link

So guys, when it's gonna be merged?

Copy link
Member

@jrudolph jrudolph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, seems weird to me that the signature implies that any kind of keys would be supported when in fact it has stricter constraints on the key format.

Here are two potential alternatives:

  • model key entries as { "key": ..., "value": ... } which would really support any kind of keys and values
  • create another type-class to model "Keyable" conversions, which would probably mean providing a bidirectional conversion K <=> String

Copy link
Member

@ktoso ktoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking as not ready to merge yet; has review feedback to be addressed

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.

5 participants