-
Notifications
You must be signed in to change notification settings - Fork 18
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
contributing.md #55
contributing.md #55
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.
This change isn't necessary
CONTRIBUTING.md
Outdated
iOS: | ||
XCode | ||
Node & Watchman using Homebrew. | ||
- brew install node | ||
- brew install watchman | ||
Cocopod | ||
- $ sudo gem install cocoapods. | ||
|
||
Andriod: | ||
Android development environment. | ||
Java Development Kit - Zulu using Homebrew. |
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 isn't necessary. These sorts of packages all assume that users have already set up a react native development environment.
Also note: the "issue" work item complains about another issue. |
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 change isn't really necessary -- as noted, the inserted lines are not needed. Should we close this PR or were there other changes you wanted to make?
I will make changes. I got some answers from Robert.
Get Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: Charles P ***@***.***>
Sent: Tuesday, April 23, 2024 10:26:55 AM
To: cdiddy77/react-native-mediapipe ***@***.***>
Cc: sukanvisapearyoo ***@***.***>; Author ***@***.***>
Subject: Re: [cdiddy77/react-native-mediapipe] contributing.md (PR #55)
@cdiddy77 requested changes on this pull request.
This change isn't really necessary -- as noted, the inserted lines are not needed. Should we close this PR or were there other changes you wanted to make?
—
Reply to this email directly, view it on GitHub<#55 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/A326HSIANS4N2HAUWEO77HTY62KV7AVCNFSM6AAAAABGQCORBCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMJXHA3DCMJRGE>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
CONTRIBUTING.md
Outdated
@@ -37,7 +24,7 @@ Read the READMEs in [`android/`](android/README.md) and [`ios/`](ios/README.md) | |||
|
|||
### iOS | |||
|
|||
1. Open the `react-native-mediapipe/example/ios/MediapipeExample.xcworkspace` file with Xcode | |||
1. Open the `react-native-mediapipe/examples/ios/MediapipeExample.xcworkspace` file with Xcode |
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 is still not the correct path - there is a folder called objectdetection under /examples/ and the xcworkspace file is called differently.
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.
LGTM
docs(contributing): Modified the 'Get Started' section by adding iOS and android 'Install dependencies' part