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

Rename things for Ably #8

Closed
wants to merge 2 commits into from
Closed

Rename things for Ably #8

wants to merge 2 commits into from

Conversation

lawrence-forooghian
Copy link
Collaborator

What's new

This is a preparatory step on the road to compiling SocketRocket directly into ably-cocoa, as described in this comment on ably/ably-cocoa#969.

It changes the naming so that things look like Ably code - specifically, replacing the SR namespace with ARTSR.

With this change, an application could in theory use both our SocketRocket fork and its own version of SocketRocket without experiencing any issues.

Notes

Normally, in a fork, a big rename like this would be something to be very careful about because it makes it very difficult to merge further changes from upstream. But the upstream project is well and truly dead, so I don't think we need be concerned.

Questions for reviewers

  • What's the best way to test this? I tried locally changing the Cartfile in ably-cocoa to point to this changed version, and I had 3 failing tests when running the tests inside Xcode 12. But I don't know whether that's to be expected or not. And I also pushed it to a private CocoaPods repository and checked that I was able to compile an app that uses this version of the library.
  • Do we need to bump the library version number now? Should that be part of this PR?

What's next

  • Once this is merged and the version number bumped, open a PR on ably-cocoa to update the SocketRocket dependency version.
  • After that, continue with the plan described in the aforementioned comment on Remove develop branch ably/ably-cocoa#969, by compiling this library directly into the SDK.

Using the regexp /(?<!ART)SR(?!unLoop)(?!ange)/.

And then handle the consequent changes that this requires - namely,
renaming files and updating the README.
This in particular affects the framework bundle identifiers, so that
our fork can coexist alongside the Facebook version.
@lawrence-forooghian lawrence-forooghian deleted the 969-rename-things-for-ably branch October 22, 2020 21:54
@lawrence-forooghian
Copy link
Collaborator Author

Oops, I got the issue number wrong in the branch name and PR description. It should be 949, not 969. I've replaced this PR with #9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant