std.json broke backward compatibility, starting with https://github.com/D-Programming-Language/phobos/commit/1958c95666b0241d669d282806e4f724fbb37caf. There's even no notice in the changelog of the breakage, which is quite obvious by inspecting the diff.

This is a serious matter because it broke production code. Was there a reason for the breakage? We should make sure breaking changes are avoided or get a lot of scrutiny if they are really necessary.

I wonder how we can improve the process to avoid such issues in the future.


Andrei

On Sunday, 9 March 2014 at 03:58:31 UTC, Andrei Alexandrescu wrote:
> std.json broke backward compatibility, starting with https://github.com/D-Programming-Language/phobos/commit/1958c95666b0241d669d282806e4f724fbb37caf.

Pull request link with discussion:
https://github.com/D-Programming-Language/phobos/pull/1421

> There's even no notice in the changelog of the breakage, which is quite obvious by inspecting the diff.

Changelogs should be generated from pull requests, not Bugzilla issues anyway.
https://d.puremagic.com/issues/show_bug.cgi?id=12240

> This is a serious matter because it broke production code. Was there a reason for the breakage? We should make sure breaking changes are avoided or get a lot of scrutiny if they are really necessary.

I don't think it was an intentional change, rather a mundane regression. Could you post the affected code?

> I wonder how we can improve the process to avoid such issues in the future.

Make it easy for individuals and companies to set up CI servers which automatically test their projects with beta versions of D?

On 3/8/2014 8:08 PM, Vladimir Panteleev wrote:
> On Sunday, 9 March 2014 at 03:58:31 UTC, Andrei Alexandrescu wrote:
>> I wonder how we can improve the process to avoid such issues in the future.
>
> Make it easy for individuals and companies to set up CI servers which
> automatically test their projects with beta versions of D?

The coverage of the unittests for std.json is 92%. But most of the uncovered lines should be trivial to cover with unittests. For example, JSON_TYPE.UINTEGER is completely untested.

A review of unittest coverage should be part of any review of new library code.

Note that checking the unittest coverage is as easy as:

    dmd std/json -unittest -main -cov

BTW, I don't know if better unittest coverage would have detected this particular breakage, but in any case, we should do better with coverage.

Am 09.03.2014 04:58, schrieb Andrei Alexandrescu:
> std.json broke backward compatibility, starting with
> https://github.com/D-Programming-Language/phobos/commit/1958c95666b0241d669d282806e4f724fbb37caf.
> There's even no notice in the changelog of the breakage, which is quite
> obvious by inspecting the diff.
> 
> This is a serious matter because it broke production code. Was there a reason for the breakage? We should make sure breaking changes are avoided or get a lot of scrutiny if they are really necessary.
> 
> I wonder how we can improve the process to avoid such issues in the future.
> 
> 
> Andrei

What code did break?
I did use std.json heavily myself before this pull request and it was
basically unusable (hence the pull to improve the api), so I tried to
expand std.json without breaking backwards compatability, the tests ran
through also my code still worked.

On 3/9/14, Vladimir Panteleev <vladimir@thecybershadow.net> wrote:
> I don't think it was an intentional change, rather a mundane regression. Could you post the affected code?

Breaking code wasn't intended.

On 3/9/14, Andrei Alexandrescu <SeeWebsiteForEmail@erdani.org> wrote:
> I wonder how we can improve the process to avoid such issues in the future.

The obvious answer is to have proper unittests. If the API broke, the existing tests should have caught this. But the tests weren't touched in the pull request, only new ones were added. What exactly broke though?

On Sunday, 9 March 2014 at 12:35:11 UTC, David wrote:
> What code did break?

It made the type field a read-only thing which broke building json structures.

On 3/9/14, Walter Bright <newshound2@digitalmars.com> wrote:
> BTW, I don't know if better unittest coverage would have detected this particular breakage, but in any case, we should do better with coverage.

Can someone finally say *what* broke?

On 3/9/14, 5:35 AM, David wrote:
> Am 09.03.2014 04:58, schrieb Andrei Alexandrescu:
>> std.json broke backward compatibility, starting with
>> https://github.com/D-Programming-Language/phobos/commit/1958c95666b0241d669d282806e4f724fbb37caf.
>> There's even no notice in the changelog of the breakage, which is quite
>> obvious by inspecting the diff.
>>
>> This is a serious matter because it broke production code. Was there a
>> reason for the breakage? We should make sure breaking changes are
>> avoided or get a lot of scrutiny if they are really necessary.
>>
>> I wonder how we can improve the process to avoid such issues in the future.
>>
>>
>> Andrei
>
> What code did break?
> I did use std.json heavily myself before this pull request and it was
> basically unusable (hence the pull to improve the api), so I tried to
> expand std.json without breaking backwards compatability, the tests ran
> through also my code still worked.

As is plain from the diff, the publicly available storage now needs ".store" in there. Making untagged store public was probably a mistake in the initial design, but breakage is what it is.

Andrei

On 3/9/14, 6:23 AM, Andrej Mitrovic wrote:
> On 3/9/14, Vladimir Panteleev <vladimir@thecybershadow.net> wrote:
>> I don't think it was an intentional change, rather a mundane
>> regression. Could you post the affected code?
>
> Breaking code wasn't intended.
>
> On 3/9/14, Andrei Alexandrescu <SeeWebsiteForEmail@erdani.org> wrote:
>> I wonder how we can improve the process to avoid such issues in the future.
>
> The obvious answer is to have proper unittests. If the API broke, the
> existing tests should have caught this. But the tests weren't touched
> in the pull request, only new ones were added. What exactly broke
> though?

Apparently direct use of the untagged union elements was not being unittested.

Andrei