More deprecation warnings and less breaking changes in google-cloud?

[richardkazuomiller]

I updated to the latest version of the datastore module today because of the v1beta3 deprecation, but I ran into some issues due to breaking changes from a few versions ago. I usually end up skipping a few versions at a time and only upgrade when I need to. It's difficult and time consuming to try to avoid this, because users have to go through every changelog looking for breaking changes even if it's an incremental update.

The particular issue I ran into was runInTransaction not working anymore. It would have been nice to have the function work but log a deprecation warning before eventually being removed, instead of finding out after deploying to production. I think wouldn't be difficult to include something like this:

datastore.runInTransaction = (stuffToDo,callback) => {
  console.log('runInTransaction is deprecated!')
  const transaction = datastore.transaction()
  transaction.run((err) => {
    if(err) {
      callback(err)
    }
    stuffToDo(transaction,() => {
      transaction.commit(callback)
    })
  })
}

Since I was using what was the latest version just three months ago, I didn't expect such a major change without any warning. In other languages, if a function is removed after upgrading a dependency, a compile time error will let the developer know, but since JavaScript doesn't have anything like that and we can't tell if code is broken until it runs, it's even more important here.

Another issue I've had is with too many things being changed at the same time. When I started using Datastore, my app was running on Node v0.10 (which is still being maintained). When the Datastore v1beta1 API was deprecated, I found out that I couldn't use Node v0.10 anymore, because gcloud-node wasn't compatible. I upgraded to Node v4 and found that the API had also been changed significantly, so it turned into a very large refactor. It would be helpful if in the future, google-cloud packages were maintained so upgrading doesn't require as much refactoring.

Now that Datastore is out of beta, can we expect to see some more stability in the functionality of google-cloud?

[stephenplusplus]

I'm sorry that the changes made for a difficult time. The changes we've introduced since we first introduced gcloud-node to npm have definitely been breaking. Some have affected lesser known methods and features, others were more noticeable. We definitely should have considered a gradual deprecation plan instead of the abrupt pattern we've been using so far.

As implied by semver, while we're pre-1.0, we've guaranteed no stability. As soon as a better idea or new feature comes along that needs to sunset another, we waste no time in bettering our API. We communicate breaking changes in our release notes big and bold, front and center. The complexity of dragging along legacy code while still pre-1.0 is what has discouraged me from a gradual deprecation plan. However, it's clear from this issue that some legacy code is worth it to prevent having an issue like this again.

All I can say at this point is I'm sorry that we've made using this library harder than it should be. I'm going to reconsider our future releases and make sure our breaking changes are handled more delicately. Thanks for prevailing through the frustration and letting us know about it. Feel free to do that anytime, so we can continue to improve.

[richardkazuomiller]

Thanks. I hope I wasn't too critical. Overall you all are doing great work d(^o^)

[stephenplusplus]

Not at all. I know you've been using this library for a long time, so your feedback is very meaningful.

[stephenplusplus]

Trying out another way of communicating upcoming changes to users: https://twitter.com/stephenplusplus/status/770295145241640961

This one being discussed is small enough that it should impact a low amount of users.

[richardkazuomiller]

Thanks. To be honest though, I think tweets are too easy to miss. I usually don't look at tweets more than two days old, and I usually only update my dependencies when I need to, which is usually like once every few months.

I have an idea that I think will improve the taxonomy of release notes. How about an issue for each breaking change with a tag "breaking-change" and a tag for the package so we can just view all the issues with those tags to get all the information we need? Since the issues are sorted by date, we can just start at the top and read down until the last time we updated.

Speaking of which, since the packages for each API are going to be released separately from now on, where are the release notes going to be? I think a tag/release in the repository doesn't make as much sense now because, for example, a update to translate won't affect people who use data store, and the more releases there are of other packages the harder it is to find data store release notes from several months in the past. Creating an issue for release notes might be good in the future.

A page in the wiki for each package could also work.

[stephenplusplus]

How about an issue for each breaking change with a tag "breaking-change"

Could the tag just go on the issue or PR that describes the change, as opposed to another issue?

since the packages for each API are going to be released separately from now on, where are the release notes going to be?

We have git tags for each release (v0.38.0 & bigtable-0.1.0 for example). We've yet to come upon our first incremental release since the first batch of modularized releases, so the plan was to create release notes for "@google-cloud/bigtable 0.2.0" as well as "google-cloud 0.39.0". Each release (modular and bundled) get their own release notes.

Creating an issue for release notes might be good in the future.

Can you elaborate on this approach? At a glance, it doesn't seem more intuitive for a user to find release notes here instead of under the release notes tab. In my experience as a user, the release notes tab has been sufficient with other projects.

Overall, I think the breaking-change label is a great idea. Maybe instead of labels for each version, we could start using milestones to bundle related changes.

// cc @callmehiphop @jgeewax @omaray

[stephenplusplus]

PS: Thanks for taking the time to help us out!

[richardkazuomiller]

Could the tag just go on the issue or PR that describes the change, as opposed to another issue?

The biggest problem with just using the issue/PR that describes the change is that there is usually a lot of discussion involved so there's a lot of stuff to read through before we can find the actual part that describes what will be broken in a future release, and GitHub collapses comments so in #1524 the comment where you specify the change is actually hidden by default and easy to miss.

We have git tags for each release

What I don't like about git tags being the home for release notes, especially now, is that every modularized release is in the same list. When I updated from v0.34 to the latest modularized version, I had to go through four pages to find the release notes for v0.37 which contained the description of the breaking change. The release notes of other modules made it harder to find the information I needed about Datastore because we have to skim past the documentation for everything other than the module we're updating. Since as far as I know, there isn't a way to filter releases by module on GitHub, this could get worse over time as modules get updated more, and possibly more modules are added.

I thought issues would be good just because they're easy to create and filter by tag, but it is counterintuitive so maybe they aren't the best place to keep release notes. What about including changes in the README or in a RELEASE_NOTES.md file in each module that looks something like this?

Datastore Release Notes

datastore-0.1.1

Fixed publish script.

datastore-0.1.0

Datastore is now its own module, @google-cloud/datastore!

🎉 Now you can install only the APIs you need

$ npm install --save --save-exact @google-cloud/datastore

var datastore = require('@google-cloud/datastore')();

No more wasting time installing dependencies from the other modules that your project doesn't require.

v0.37.0

⚠️ Breaking Changes

Datastore: Transaction objects

Issue: #633
PR: #1369

Datastore Transactions are now their own custom type. Previously, operations have been blanketed by an "all-in-one" API. This led to some confusion about which steps were taking place behind the scenes; especially in the case of an error-- e.g., which part failed? See the example below for the differences.

Before

datastore.runInTransaction(function(transaction, done) {
  transaction.save({
    key: key,
    data: { value: true }
  });

  done();
}, function(err) {
  // Where is this error coming from?
});

After

var transaction = datastore.transaction();

transaction.run(function(err) {
  if (err) {
    // Error handling omitted.
  }

  transaction.save({
    key: key,
    data: { value: true }
  });

  transaction.commit(function(err) {
    if (!err) {
      // Transaction committed successfully.
    }
  });
});

v0.36.0

Fixes

(#1357, #1358): Allow setting maxRetries for failed requests.

... and so on.

---

This way, all the changelogs of a particular module all live in one place and aren't mixed in with those of any other packages.

[jmuk]

cc: @omaray

[stephenplusplus]

A lot has changed on our repo since this discussion-- we have moved all APIs out to their own repositories. There is more involvement with each client API from the product API teams. They have a very careful approach to enhancements and breaking changes, and a release cycle (from alpha, beta, to GA) that matches direct customer requests and needs. Going forward from here, I don't anticipate any reckless maintenance behavior.

Thanks again for all of the feedback.