<div dir="ltr">I'm with Michael on +1 as-is. <div><br></div><div>I'm rather neutral on removing the instance as it is a matter of taste and I tend to prefer these sorts of things to be simple up-down votes of confidence rather than have them devolve into long unproductive bikeshedding discussions.</div>
<div><br></div><div>-Edward</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Sep 18, 2013 at 3:22 AM, Michael Snoyman <span dir="ltr"><<a href="mailto:michael@snoyman.com" target="_blank">michael@snoyman.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div class="im">On Wed, Sep 18, 2013 at 10:09 AM, Simon Hengel <span dir="ltr"><<a href="mailto:sol@typeful.net" target="_blank">sol@typeful.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>On Wed, Sep 18, 2013 at 09:02:16AM +0200, Bas van Dijk wrote:<br>
> On 18 September 2013 06:30, Niklas Hambüchen <<a href="mailto:mail@nh2.me" target="_blank">mail@nh2.me</a>> wrote:<br>
> > On 18/09/13 04:02, Dan Burton wrote:<br>
> >> Everybody already uses, trusts, and loves aeson.<br>
> ><br>
> > There's only one thing about it that I don't trust:<br>
> ><br>
> > "Partial instances" that will call error in pure code and crash my program.<br>
> ><br>
> > ByteString has such an instance that simply assumes that ByteString is<br>
> > the same as Text, so it calls Data.Text.Encoding.decodeUtf8 on it.<br>
> ><br>
> > <a href="https://github.com/bos/aeson/issues/126" target="_blank">https://github.com/bos/aeson/issues/126</a><br>
><br>
> What about settling that issue by adding documentation to the ToJSON<br>
> ByteString instance and the 'encode' function warning users about<br>
> this?<br>
<br>
</div>That looks scary. I'd either make it total or remove the instance all<br>
together.<br>
<br>
I think by now almost everybody in the Haskell community agrees that we<br>
do not want pure partial functions.<br>
<br></blockquote><div><br></div></div><div>I don't want this issue to block inclusion of aeson. And IMO, the HP review process shouldn't be a time to try and twist a maintainer's arm into making a change he/she isn't interested in. That happened quite a bit on the text package, and I don't want to see it happen on aeson as well (especially given that Bryan would be the victim in both cases).</div>
<div><br></div><div>That said, I personally would prefer removing the ToJSON/FromJSON instances for ByteString. We've been bitten by this in our codebases, and have resorted to using a ByteString64 newtype wrapper, which uses base64 encoding to ensure safe roundtripping.</div>
<div><br></div><div>So: +1 from me on including aeson as-is, and if Bryan's interested in making this change to aeson, I'd consider it a perk.</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>Michael </div>
</font></span></div></div></div>
<br>_______________________________________________<br>
Libraries mailing list<br>
<a href="mailto:Libraries@haskell.org">Libraries@haskell.org</a><br>
<a href="http://www.haskell.org/mailman/listinfo/libraries" target="_blank">http://www.haskell.org/mailman/listinfo/libraries</a><br>
<br></blockquote></div><br></div>