search

aristidb / aws

Amazon Web Services for Haskell
BSD 3-Clause "New" or "Revised" License
239 stars 108 forks source link

DynamoDb: Avoid overlapping ToJSON instance #208

Closed bgamari closed 8 years ago

bgamari commented 8 years ago

Aeson 1.0 will provide a ToJSON [a] instance, which the previous ToJSON [AttributeUpdate] instance will overlap with. This internally wraps [AttributeUpdate] in a newtype.

An alternative here would be to expose AttributeUpdates with a singleton function and a Monoid instance

aristidb commented 8 years ago

So this is a breaking change, right?

CC @ozataman

ozataman commented 8 years ago

It may actually not be a breaking change - it looks like the newtype wrapper is only used by the ToJSON instance to facilitate serialization. The user facing interface seems to have remained the same. Right, @bgamari ?

bgamari commented 8 years ago

Ozgun Ataman notifications@github.com writes:

It may actually not be a breaking change - it looks like the newtype wrapper is only used by the ToJSON instance to facilitate serialization. The user facing interface seems to have remained the same. Right, @bgamari ?

Correct. The newtype is only used internally. I had considered exposing it though. It depends upon what others would prefer.

aristidb commented 8 years ago

AttributeUpdates became un-exported, but I guess that is a fairly minor "breaking change".

ozataman commented 8 years ago

At a quick glance, my own view is that there doesn't seem to be a clear reason for exposing it. I'd suggest keeping the simpler list of updates interface currently in there. One less type for people to worry about.

That is unless I'm missing a reason for people wanting to use ToJSON for the list wrapper directly...

bgamari commented 8 years ago

At a quick glance, my own view is that there doesn't seem to be a clear reason for exposing it. I'd suggest keeping the simpler list of updates interface currently in there. One less type for people to worry about.

This was ultimately my reasoning as well.