search

ably / specification

The Ably features spec for client library SDKs.
Apache License 2.0
0 stars 4 forks source link

DeviceDetails.id (PCD2) should be nullable. #180

Open maratal opened 8 months ago

maratal commented 8 months ago

Currently it is declared as non-nil for some reason:

class DeviceDetails: // PCD*
  id: String // PCD2

which leads to the bunch of problems and inconsistencies in the SDK(s) implementations.

sync-by-unito[bot] commented 8 months ago

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-4038

lawrence-forooghian commented 7 months ago

Alternatively we can update the spec so that it reflects the behaviour implemented in https://github.com/ably/ably-cocoa/pull/1847, where a new device ID is generated as soon as the previous ones are cleared, so that id is never null.

maratal commented 7 months ago

Alternatively we can update the spec so that it reflects the behaviour implemented in ably/ably-cocoa#1847, where a new device ID is generated as soon as the previous ones are cleared, so that id is never null.

It's an idea. But I think it's better to eventually make nullable id nevertheless. And schedule this change together with the v2 protocol. WDYT?

lawrence-forooghian commented 7 months ago

But I think it's better to eventually make nullable id nevertheless.

Why do you think it better?

And schedule this change together with the v2 protocol. WDYT?

What's the relation with the v2 protocol?

maratal commented 7 months ago

Why do you think it better?

Hmm, good question. At least it's nullable in other SDKs already.

What's the relation with the v2 protocol?

I mean v2 release is a breaking change and release of another breaking change together would make sense I think.

lawrence-forooghian commented 7 months ago

At least it's nullable in other SDKs already.

By "other SDKs" do you mean just ably-java (which is currently the only other SDK to have a LocalDevice), or are there others? And in the case of ably-java I'm not really sure what it means to say that it's "nullable". Sure, the SDK sets it to null sometimes, but there's no language-level concept of nullability, and there's nothing in the documentation to suggest to the user that they can expect it to be null.

I mean v2 release is a breaking change

The changes introduced by version 2.0.0 of the specification, which is the version that switched to using protocol v2, do not include any breaking API changes.

maratal commented 7 months ago

The changes introduced by version 2.0.0 of the specification, which is the version that switched to using protocol v2, do not include any breaking API changes.

You are right, the one that was deleted is marked as deprecated. Ok, will change the specs then.