Update readme and dependencies #21
Conversation
…id build regarding RN new architecture
…ation instructions
| @@ -27,7 +27,7 @@ | |||
| }, | |||
| "peerDependencies": { | |||
| "@daily-co/react-native-daily-js": "^0.76.0", | |||
| "@daily-co/react-native-webrtc": "118.0.3-daily.3", | |||
| "@daily-co/react-native-webrtc": "118.0.3-daily.4", | |||
There was a problem hiding this comment.
Updating this dependency as the previous version was throwing an error regarding this issue:
jiangdongguo/AndroidUSBCamera#728
There was a problem hiding this comment.
We might want to look into a CI/CD pipeline that would run a daily/weekly job to detect version updates in NPM repository
README.md
Outdated
| ```groovy | ||
| minSdkVersion = 24 | ||
| ``` | ||
|
|
||
| (If you run into any issues, refer to [Github issues](https://github.com/react-native-webrtc/react-native-webrtc/issues/720) like [these](https://github.com/jitsi/jitsi-meet/issues/4778), or the `react-native-webrtc` [installation docs](https://github.com/react-native-webrtc/react-native-webrtc/blob/master/Documentation/AndroidInstallation.md), which walk you through a more complicated process. The simpler process laid out above seems to work in a vanilla modern React Native CLI-based setup). | ||
| If you are running into issues with TurboModules, you might need to disable `newArchEnabled` in your `android/gradle.properties` file. |
There was a problem hiding this comment.
react-native-webrtc does not support React Native new achitecture, so I'm adding this instruction in order to guide users that are unintentionally on the new architecture and still need to use our SDK.
It seems that the maintainers will start working on supporting the new architecture as this issue suggests: react-native-webrtc/react-native-webrtc#1722
There was a problem hiding this comment.
Yeah, we don't care about supporting Hermes right now. IMHO this should probably be highlighted at the top of our README
README.md
Outdated
| await vapi.start({ | ||
| model: { | ||
| provider: 'openai', | ||
| model: 'gpt-4', |
README.md
Outdated
| vapi.setMuted(true); | ||
| vapi.isMuted(); // true | ||
| # Reinstall dependencies | ||
| rm -rf node_modules && npm install |
There was a problem hiding this comment.
A bit of a nit, but might be a good idea to also include cleaning npm cache. I've seen this fixing local RN builds
There was a problem hiding this comment.
It seems that npm cache clean throws an error and requires a --force flag in order to execute.
Following NPM suggestion, we can instead safely execute npm cache verify which should be enough for verifying cache integrityl
|
|
||
| You can listen to the following events: | ||
| > **Warning**: This SDK cannot be used with Expo Go. Use Expo Development Build or EAS Build. |
There was a problem hiding this comment.
IMHO this one is located too deep into the README. I'd duplicate at the top of README
There was a problem hiding this comment.
There is a warning regarding this limitation in the Important Notes part, does it looks good enough?
README.md
Outdated
| MIT License | ||
|
|
||
| Copyright (c) 2023 Vapi Labs Inc. | ||
|
|
||
| Permission is hereby granted, free of charge, to any person obtaining a copy | ||
| of this software and associated documentation files (the "Software"), to deal | ||
| in the Software without restriction, including without limitation the rights | ||
| to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
| copies of the Software, and to permit persons to whom the Software is | ||
| furnished to do so, subject to the following conditions: | ||
|
|
||
| The above copyright notice and this permission notice shall be included in all | ||
| copies or substantial portions of the Software. | ||
|
|
||
| THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
| IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
| FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
| AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
| LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
| OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
| SOFTWARE. | ||
| ``` |
There was a problem hiding this comment.
Since you removed MIT license from README, please make sure we have a LICENSE file
There was a problem hiding this comment.
Oops sorry, I didn't review this part. Cursor basically changed the license type by itself 😄
I'm reverting this part.
README.md
Outdated
|
|
||
| ## 📄 License | ||
|
|
||
| ISC License - see LICENSE file for details. |
… cache cleanup, move warnings to the Important Notes section
|
i have a question: does the sdk work with expo53 yet (i know it requires custom code) I have been building an app, and have been stuck for quite some time because I had to downgrade my project to expo 50 to have the demo working on the simulator. It never works when on a real device though. Anything I can do to help update the SDK to expo53? or the compatibility is just not there yet? |
This MR updates README for improved readability and update/add missing information
It also updates
@daily-co/react-native-webrtcversion to118.0.3-daily.4as the previous version has invalid dependencies and it is not working for fresh installs anymore.