Skip to content

Conversation

@bigosmallm
Copy link

@bigosmallm bigosmallm commented May 10, 2017

Fixes #196

name: 'enteredName',
message: 'What should your Android Studio and Xcode projects be called?',
default: newName,
validate: s => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO arrow function has a more simple form like

s => s.length > 0 && s.indexOf('-') === -1 && s.indexOf(' ') === -1

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While valid, I think this is unrelated to this PR. I think your fix can come in as a separate PR.
(Thanks for the review)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for replying. So should I open a PR for the changes after this PR is merged?

Copy link

@amingilani amingilani May 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@duchuyctlk or you could open a PR for bigosmallm/create-react-native-app:master, which when approved will be reflected here ;)
As part of this pull request

@brentvatne brentvatne self-assigned this May 12, 2017
@brentvatne
Copy link
Member

Thanks! I have a pretty busy week coming up with React Europe but this is on my radar

@Zomis
Copy link

Zomis commented Jul 31, 2017

Please, oh please, prioritize this. It's been more than three months (not years, sorry) now without a comment on this PR. In today's world, automation should be a top priority for any decent framework.

@anp
Copy link
Contributor

anp commented Jul 31, 2017

@Zomis where are you getting three years? This is ~2 months old.

Also for what it's worth I don't think that automating eject in CI is very well aligned with the reasons we originally built CRNA: to make it easier to learn React Native. Don't get me wrong, it's really awesome that people are able to build whole apps with it and to then run their native build in CI, but I feel like we should have a better solution for "production" CRNA projects.

@bigosmallm
Copy link
Author

bigosmallm commented Jul 31, 2017

@dikaiosune This is not for building 'production' apps. I need a develop -> build -> deploy cycle for a CRNA app. In my environment, the only way to get an app onto a device is to sign it using an enterprise certificate. That process has been automated. This step only makes it easier to plug the CRNA into such an environment.

I agree with you that this should not be used to build 'production' apps.

@brentvatne
Copy link
Member

I think a better solution here is to refactor react-native-scripts/src/scripts/eject.js such that it exports a function that you can call directly to eject, then exporting that from the main package so you can add a script to your project to do the eject for you.

it would look like:

import { eject } from 'react-native-scripts';

eject({
  projectName: '..',
  otherThing: '..',
});

what do you think @bigosmallm?

@bigosmallm
Copy link
Author

That should work as well. That sounds a more involved changes to eject.js. If you are going to do that, that would be great!

@brentvatne
Copy link
Member

I am not going to work on it, but anyone who would like this feature is welcome to implement it as described above and I will accept a pr for it.

I really appreciate you putting it together and I'm very sorry that it took me so long to get a response to you on it. I manage a lot of different projects and when I'm unsure about whether I'd like to add something to a project or not I have a tendency to ignore the issue, I need to get better at that. I think that the non-interactive eject option as originally proposed adds a maintenance burden on CRNA maintainers and is a niche option that only few people will actually use, and I should have expressed this sooner and suggested an alternative proposal as I did today, above. Hopefully someone implements the proposal, until then I will close this PR.

@brentvatne brentvatne closed this Jul 31, 2017
@bigosmallm
Copy link
Author

No problem. Thanks for the response.

@Zomis
Copy link

Zomis commented Aug 1, 2017

@dikaiosune Whoops, "three months" of course. Self-note: Don't comment on github PRs right before going to sleep.

@brentvatne @bigosmallm Given the eject documentation: "Warning: Running eject is a permanent action". I thought "Well, I'll just let Jenkins do it and then it is not permanent". I also did not intend to use this for production. If ejecting is a permanent action and should not be done by Continuous Integration, then when should I eject? I would recommend documenting the "recommended workflow" better. All I wanted to do was to automatically build a prototype of my app for my phone (I'm even planning to use ./gradlew assembleDebug after automatically ejecting) so that I could bring it with me without having my computer at home running npm start.

@Zomis
Copy link

Zomis commented Aug 3, 2017

@dikaiosune @brentvatne @bigosmallm See my Pull Request #346 . I spent some time learning more JavaScript only to be able to give you this pull request. It was educational.

@Zomis
Copy link

Zomis commented Sep 6, 2017

Ping @brentvatne . It's been more than a month now since I added a PR.

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.

6 participants