Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RTN2g #210

Merged
merged 2 commits into from
Nov 11, 2020
Merged

Add RTN2g #210

merged 2 commits into from
Nov 11, 2020

Conversation

gernest
Copy link
Contributor

@gernest gernest commented Nov 11, 2020

@SimonWoolf the library name is ably-go and not just go so the lib query param will contain something like ably-go-1.1.5 instead of go-1.1.5

@gernest gernest requested a review from tcard November 11, 2020 17:18
@SimonWoolf
Copy link
Member

the library name is ably-go and not just go

I mean, ably-js, ably-ruby, ably-python etc. are all called that, but still identify themselves as lib=js-web-xxx, lib=ruby-xxx, lib=python-xxx, per the spec.

But whatever, that doesn't matter all that much, the most important thing is that it's identifying itself somehow

@tcard
Copy link
Contributor

tcard commented Nov 11, 2020

the library name is ably-go and not just go

This isn't right according to RSC7b, which says that for ably-X, lib is X. Can we make that change? This should also affect the X-Ably-Lib header so it's just a matter of removing ably- from proto.LibraryName.

@gernest
Copy link
Contributor Author

gernest commented Nov 11, 2020

@tcard @SimonWoolf I checked the spec you are right. I have made the change to use go instead of ably-go PTAL

Copy link
Contributor

@tcard tcard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimonWoolf Do you need this backported to the 1.1 branch (main) and released?

@SimonWoolf
Copy link
Member

Do you need this backported to the 1.1 branch (main) and released?

Given we're releasing ably-go 1.2 soon, do we expect significant numbers people to be shunning that and upgrading only to the newest 1.1.x? If yes then yes, else no.

@gernest gernest merged commit 363dabe into integration/1.2 Nov 11, 2020
@gernest gernest deleted the feature/rtn2g branch November 11, 2020 18:01
@tcard
Copy link
Contributor

tcard commented Nov 12, 2020

Given 1.2 will come not just with an API upgrade but with major, critical features, I'd expect we'll be pushy about migrating users to 1.2.

lmars added a commit to ably/ably-common that referenced this pull request Jan 9, 2023
To track old ably-go versions which set `X-Ably-Lib` to
`ably-go-<version>` rather than `go-<version>`, which was changed in Nov
2020: ably/ably-go#210

Fixes #179.

Signed-off-by: Lewis Marshall <lewis.marshall@ably.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants