-
Notifications
You must be signed in to change notification settings - Fork 2
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before merging lets run e2e tests on react-native-cliend-sdk repo.
Another thing, yesterday you said that websocket client is not documented properly. Maybe we should change websocket package for another??
FishjamClient.podspec
Outdated
|
||
Pod::Spec.new do |s| | ||
s.name = 'FishjamClient' | ||
s.version = '0.3.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets have fresh start and change version to 0.1.0 or 1.0.0 if namespaces are changed and e2e tests from react-native repo pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idk, this will make no real difference here. And repo already have 0.2.0 released. So I would have to remove old releases.
As for 1.0.0 - this SDK is not ready to have v1 released
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That release will not work because url and package name changed. It is strange for first package release to be versioned as 0.3.0 instead of 0.1.0 or 0.0.1. Release 0.2.0 should be removed because we do not want old releases here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That release will not work because url and package name changed
I don't understand - what will not work in this release? It is already working.
Release 0.2.0 should be removed because we do not want old releases here
What is the problem with old release? It is kind of historical release, and I don't see any issue with keeping it here. Between 0.2 and 0.3 we are just adding some new functionality into lib, it is not complete rewrite.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, let it stay
With all these changes, for me it makes much more sense to just run apps manually and do some manual testing for joining/disconnecting. There is a lot of things that can get broken during refactors, so we should do more detailed testing once we finish all refactors.
W can use any other lib - I don't have any preferences. I was finally able to migrate existing library. So let's how it is still working. If not - we can update it in next step. |
c18a399
to
e4bac22
Compare
It can be manual testing but in this package example is small and incomplete. One resonable example app is in rn-client-sdk. If you link this repo in podfile and then test it manually it is great.
If nothing breaks it can stay. We will see that in future. |
This change is already linked with rn-client-sdk. TBH example in this project has very similar functionality as the one in RN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
This PR prepares code to be released as a pod.
Starscream
andSwiftPhoenixClient
to latest version