Bootstrap as an option for generator

[yorzi]

Really busy in whole last week, only start to have some time for #96

It's not fully ready yet. But I hope to have this PR submitted for people to review and give me feedback, so that I am able to make it better. I still need to adjust/add some rspec testings.

[justin808]

@robwise Please take a look at this and see if this should go into v1.1.0.

[justin808]

@robwise @yorzi We're getting some build failures.

[robwise]

I talked to @justin808 and he said he'd prefer if this would default to true

[yorzi]

@robwise let me make it default to true
@justin808 I will let this PR pass build today.

[justin808]

@yorzi, let @robwise and I know when this one is ready.

[yorzi]

@justin808 I think it's ready with correct functions and testings. The only concern for now is to document acordingly, but I am not sure where and how to do that correctly. Please take that document thing in mind before merging to your master.

[justin808]

@robwise I'll let you take a first review at this, then I'll review.

[justin808]

@robwise Let's try to get this wrapped up tomorrow. I'd like to push a version of the generator diffs (prs) to take a look at going basic -> basic + bootstrap.

[robwise]

@justin808 @yorzi this may be a little unclear, as it makes it sound as though we don't need to expose components to window when server rendering. We still need to do that, just in addition we need to expose to global.

[yorzi]

@robwise I think this doc is not supposed to be updated in this branch. It might because my editor removes an space here.. anyway, can you or @justin808 do this doc updates accordingly?

[justin808]

@yorzi or @robwise we should do something to get rid of trailing spaces.

[robwise]

Just a couple of comments, the biggest thing being using --skip-bootstrap as the option instead of --bootstrap if by default the bootstrap generator will be run. This is a Rails convention that we were copying as it hints to the user which options are on by default and which are not.

@yorzi All in all, a very good job—I'm delighted to see that you were able to read the code and change things exactly how they should have been, and without any help! I also liked how you refactored the bootstrap examples tests.

[justin808]

@robwise when you're ready for me to do a final review after you have generated the PR of basic to basic + bootstrap, let me know.

[robwise]

@justin808 I'm just confused as to what we are recommending the user should be doing when using stylesheets without bootstrap. Can they just use them the normal way you would for a Rails app where they go in app/assets/stylesheets, or alternatively, the normal way you would use webpack to require the stylesheets? Both? Are there any special caveats that they will need to watch out for or address?

[yorzi]

@robwise good points! I've updated README to include some more explanation for bootstrap option. You or @justin808 can help to reword it with more details if needed.

[justin808]

@yorzi Great job!

• rebase down to a nice commit with a nice message
• work with Rob to get yours here: https://github.com/shakacode/react_on_rails-generator-results Rob has a script that creates these. Once yours is up, I'll review the diffs.

[yorzi]

@justin808 what do you mean by "rebase down to a nice commit with a nice message" here?

@robwise can you help with the generator results or would you let me know how to do that with your script?

[justin808]

@yorzi

• http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html
• https://robots.thoughtbot.com/git-interactive-rebase-squash-amend-rewriting-history
• https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History

Rob or I can give you a hand with this.

[robwise]

@yorzi As for the generators, all you need to do is go to rakelib/examples.yml and follow the pattern there to generate an example app that passes --skip-bootstrap. If it turns out to fail the tests not because it isn't working but because something about the tests expects bootstrap, you can modify the dev_tests generator to do something special (such as put a different type of test in the spec folder) in the case that --skip-bootstrap is passed.

[yorzi]

@justin808 oh, I do know the way to rebase and combine commits/logs, so you want me to combine these commits with better git logs, right?

[justin808]

Please combine to one commit with nice description that I can use in the change log.

[yorzi]

@robwise your guide is to test dummy app, but @justin808 asked for a generator results for diff from here: https://github.com/shakacode/react_on_rails-generator-results . Do I understand something incorrectly?

[robwise]

@yorzi You're absolutely right, I misunderstood. I went ahead and took the liberty of modifying my little generator script to generate an app that uses your new option so that we can look at the diffs.

Check it out here:
https://github.com/shakacode/react_on_rails-generator-results-testing/pulls

Specifically, here is a comparison of running the generator with the default options versus passing in your new --skip-bootstrap option:
https://github.com/shakacode/react_on_rails-generator-results-testing/pulls

It looks good to me, what about you?
cc: @justin808

[robwise]

Also I configured rake to generate an example app for you in a branch based off of your changes: https://github.com/shakacode/react_on_rails/blob/bootstrap-as-an-option/rakelib/examples_config.yml#L18

It would probably be easiest if you just manually made this change in your own branch

[yorzi]

@robwise 👍 and I added that configuration for sample app .
cc: @justin808

[coveralls]

Coverage decreased (-52.6%) to 37.273% when pulling e560da6 on yorzi:master into 899fefc on shakacode:master.