gauravChaturvedi / react-skeleton

0 stars 0 forks source link

Potential improvements to repo #1

Open climboid opened 8 years ago

climboid commented 8 years ago

Hey Gaurav! So one thing that calls out is that you are separating the tests from the components. Its best to keep everything contained within its self. For instance in your components folder you can have header.jsx, header.scss, header.test.js. That way whenever someone looks at the component they have the entire context of that component. I don't see any tools for e2e tests; those are a bit more of a personal preference but if implemented at the right time, will save you loads of hours in manual testing. Take a look at nightwatch.js for more info on them. I second the motion of adding redux afterwards, since for some pages you might not even need it depending on how your app is designed.

Build script

Please add a build script within your package.json file that generates some kind of dist folder with all that is required inside of it (minified, concatenated etc) That way you can tie it to your Continuous Integration tool once all tests pass and have a nice package to scp to your server boxes.

Back end code

I don't see a server folder with any framework or api. You will most likely have this so you'll need to choose either between Hapi or Express or Strongloop. Please check out all of them and let me know what you think. In short Hapi is great if you're just building an API. Express is in the middle between Hapi and Strong loop because it gives you a barebones structure but you need to build everything out your self. Strongloop is like rails but for node and it will speed up your delivery process quite a bit, though lots of magic is happening behind the curtains and you need to make sure you know how that works incase things go bad and you need to debug.

gauravChaturvedi commented 8 years ago

Hi Oscar, thanks so much for this, appreciate you taking out time to review this :)

climboid commented 8 years ago

Awesome! If you're on a Java stack will you be hitting a hosted API? what is the plan to test the entire app if it's not within the same repo? All this doable and good, just making sure your strategy aligns to cover a real life scenario.

On Jun 23, 2016, at 10:40 AM, Liberty-DHub notifications@github.com wrote:

Hi Oscar, thanks so much for this, appreciate you taking out time to review this :)

Agree with you on the tests part -- makes sense to have everything related to a component in one place. Good spot on absence of e2e, I will lookup nightwatch.js (GoT reference ?) I have added a build script now.. will push it soon. I have used Hapi before, It's nice to use. On this engagement however, we are using Spring Java for Backend code.. and the team feels it is best to keep FE and BE code in separate repos. WDYT ? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

gauravChaturvedi commented 8 years ago

@climboid Yes, we will be creating APIs on our Java app that'll be hosted.. somewhere :p and will hit these APIs from our React app.

And yeah, the reason I reached out to you with this was to make sure the base is solid so that we have to make minimal changes moving forward.

gauravChaturvedi commented 8 years ago

So any other things you think I can improve upon or remove entirely, please let me know :)

climboid commented 8 years ago

Will do! Thank you for lending an ear and you guys are going to have a great time in Jo'burg. Best of luck and feel free to reach out any time.

On Jun 23, 2016, at 4:53 PM, gauravChaturvedi notifications@github.com wrote:

So any other things you think I can improve upon or remove entirely, please let me know :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.