backwards compat for Async.command_basic

[Anil Madhavapeddy]

I'm seeing a lot of fallout from this particular change in Core 111.17.00 [1]:

> Added Command.async_or_error and renamed Command.async_basic to Command.async.
>
> Command.async_or_error is similar to Command.basic and Command.async, but accepts a unit Or_error.t Deferred.t type.

While the change looks good, Command.async_basic disappearing does break existing software and forces them to pin back to an earlier Core. It's also used throughout the Real World OCaml Async chapter.

For these sorts of changes, what's the Core policy on maintaining a deprecated version of the function call for a while? (perhaps emitting a deprecation warning to stderr)

[1] https://github.com/ocaml/opam-repository/pull/2208

-anil

[Yaron Minsky]

Yeah, it's a good question. Looping in Sweeks, who made the change.
I saw this go by and probably should have pushed to think about it
more.

This is a good example because it's a very common function, used in
lots and lots of applications. Keeping around a deprecated version
seems plausible. Maybe we shouldn't have made the change to the name
at all, given that keeping around a deprecated version is pretty ugly
too.

It's worth noting that different parts of Core are at different levels
of stability, so it's hard to have a completely uniform policy, given
that there are 120 or so separate modules in Core_kernel alone, and
another 40 added by Core.

Sweeks, how would you feel about adding async_basic back in?

y

> --
> You received this message because you are subscribed to the Google Groups "ocaml-core" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to ocaml-core+...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[Nathan Linger]

I was the one that made this change. I agree that it change breaks
existing code, but it does so in a completely trivial way, right? Why
does this prevent anyone from upgrading to a newer version of Async
(this particular function is in Async.Command, not Core.Command)?

What am I missing?

[David House]

I think it just requires a lot of co-ordination from lots of people. Lots of packages have to be updated. Any package that depends on an un-updated package can also not itself update. 

[Malcolm Matalka]

On top of that, I can only have one version of Core installed at any
time and used in a project.

Backward breaking changes are extremely expensive. And can, arguably,
have a negative effect on building a community.

Also, does Core use semantic versioning? It doesn't look like this
bumped the major, even though it should have?

[Anil Madhavapeddy]

David and Malcolm are spot on: if we don't coordinate all the
changes among all the broken packages, then users will rapidly require
conflicting versions of Core for their working package set. The only
solution here is to resort to OPAM switches, which isn't ideal.

Having a few months to prod maintainers into updating to the preferred
API is preferable. Some packages (or indeed, books) will take significantly
longer to upgrade -- the Real World OCaml errata will probably only be
reflected in print towards the end of the year, at a guess.

-anil

[Yaron Minsky]

For this change, I think we can just put an alias to async_basic back
in and release it this week.

A "semantic versioning" for Core is very painful and I think we
shouldn't do it. The versions for Core are tied to our own internal
releases, and we simply don't have this kind of guarantee as part of
our system. Think of it as our development branch.

Now, I do think we can get something like "semantic versioning" in the
context of the upcoming OCaml Platform. Platform releases should be
considerably less frequent than our every-2-week release schedule for
our libraries. Once the platform is released, one would largely leave
it unchanged, with only the occasional backported bugfix, and no API
changes.

I think that should give us the stability that we need. I agree that
it will help the community greatly to do so.

The OCaml Labs folks are working hard on making it possible to create
platform releases, but we're not quite there yet. In the meantime, I
think we just need to be thoughtful about the changes we do make.

In addition, I think we should generally be more reluctant to make
breaking changes than we've been in the past, as these changes now
have broader effects. Looking back on it, I tend to think this
particular change was a mistake.

y

[benedikt....@gmail.com]

I think this just highlights a different model of software development. At JS we do changes like this to some extend all the time and rely on most of working in one big tree ‎(the truth is more complicated but has a similar end-result) to force the person doing the change to do most of the work.

The alternative proposal of leaving deprecated functions around would be more practical if one had a way of marking those such that the build system told you both:
- you just added a new call to a deprecated function
- hey you are making changes to a module containing a call to a deprecated function consider changing that as well...

Otherwise there is high chance of just ending up with both for a very long time. 

I agree however that this more painful for external users of core.

Cheers,

Bene

  Original Message  
From: Malcolm Matalka
Sent: Wednesday, 11 June 2014 14:19
To: ocaml...@googlegroups.com
Reply To: ocaml...@googlegroups.com
Subject: Re: backwards compat for Async.command_basic

[Yaron Minsky]

I'm a bit leery of having "deprecated" APIs, because of the work
implied (and the way it clashes with our internal workflows. Deleting
the API is how we get ourselves to jump to the new world.) If we get
the platform story working well, do you think it's still necessary?

y

[Yaron Minsky]

Ah, so you could imagine doing something clever in 4.02, where we put
an annotation on deprecated functions, and then we can disallow their
use internally, perhaps. That might be workable.

y

[Valentin Gatien-Baron]

I think the compiler implements [@@ocaml.deprecated] (and [@@deprecated] as a shorthand) already. I am not sure what it does though. I suppose it gives a warning when you reference something deprecated.

[Anil Madhavapeddy]

I think the answer improves as we progress through compiler revisions:

- For 4.01.0 (the current supported target), we just need to be careful
about unnecessary API breakage like this.

- For 4.02.0, there will be @@deprecated annotations that can be used
to signal that things will be removed in a few releases, and OPAM can
scan and warn about this.

- The Platform will be 4.02.0+ only, and can provide a place to put a
'long term supported' Core which is released once every (e.g.) 6 months
and has only bugfixes applied in the meanwhile. It's not clear who
will manage this branch as it's counter to the internal workflow at
JS, but we can cross that bridge when we come to it. The Platform
will at least support this workflow (as it will for Lwt, Batteries and
other standard-library-ish replacements).

-anil

[Nathan Linger]

Ok, we'll add back in the deprecated version and in the future mark it
as such in a way that tools can check.

[Jeremie Dimino]

In the meantime, I'll add it to the 111.17.00 release.

Jeremie