-
Notifications
You must be signed in to change notification settings - Fork 190
Add iOS build phase #90
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
Conversation
src/commands/init.js
Outdated
| { | ||
| type: 'input', | ||
| name: 'entry', | ||
| message: 'Enter the name of the .xcodeproj file', |
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.
Isn't this the path and not name?
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.
Yup, it's path
src/commands/init.js
Outdated
| let project = fs.readFileSync(path.join(entry, 'project.pbxproj')).toString(); | ||
|
|
||
| // Are we already integrated? | ||
| if (project.includes('AD0CE2C91E925489006FC317')) { |
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.
Let's store this in a constant
src/commands/init.js
Outdated
| entry = path.resolve(cwd, result.entry); | ||
| } | ||
|
|
||
| const progress = ora('Adding haul to your native build pipeline'); |
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.
Adding haul to your XCode build scripts
Though I feel like we should ask before this change. Looks rather invasive and I might just want to try before integrating.
src/commands/init.js
Outdated
|
|
||
| // Are we already integrated? | ||
| if (project.includes('AD0CE2C91E925489006FC317')) { | ||
| progress.info('Haul is already part of your build pipeline'); |
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.
nit: Mention 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.
build pipeline sounds very serious, maybe build scripts is better
src/commands/init.js
Outdated
| progress.succeed('Added haul to your native build pipeline'); | ||
| } else { | ||
| progress.fail( | ||
| 'Failed to add Haul to your build pipeline. See docs for manual instructions.', |
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.
nit: Mention XCode
Link to docs?
src/commands/init.js
Outdated
|
|
||
| if (sectionsCount > 0) { | ||
| fs.writeFileSync(path.join(entry, 'project.pbxproj'), project); | ||
| progress.succeed('Added haul to your native build pipeline'); |
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.
nit: Mention XCode
Issue #87
As of now, the only valid approach for integrating Haul with existing React Native applications is to define a script phase that runs every time we build our project and overwrites
packager.sh(used to automatically start packager on run) andreact-native-xcode.sh(used to bundle your app).Problem with setting env variable
CLI_PATHis that some scripts are defined inReact.xcodeproj. That means scripts defined there will not receive env variables set by parent project.Thanks to regular expressions, this is rather easy.