cam-technologies / time-booker

Web based time booking application, build using the MEEN stack. This application will allow users to manually enter work times in blocks of hours and minutes for each day of the week.
2 stars 7 forks source link

Refactor client code #9

Closed martinmicunda closed 10 years ago

martinmicunda commented 10 years ago

Refactor client code to follow this structure:

time-booker/ 
  |- client/                
  |  |- src/                 
  |  |  |- adapters/
  |  |  |- components/
  |  |  |- controllers/
  |  |- assets/
  |  |- vendor/
     etc.
madole commented 10 years ago

That refactor didn't change the directory structure of the client to look like the above. it now looks like

|-client |---src |------app |---------adapters |---------controllers |---------helpers . . . |------assets |------index.html

I think this layout is not as straight forward as the previous one.

I think we should stick to the layout of the ember app kit, but happy to discuss if there are reasons for changing it.

chrislaughlin commented 10 years ago

Yeah I dont think the commit for the changes was in the pull request. I think it should be something like this:

|-client |---src |------app |---------adapters (Services etc ) |---------controllers (Controllers) |---------helpers (Helper functions/Globals) |------assets (bower compents) |------index.html |---test (Unit and SCN tests)

What do you think, the ember starter app was a bit over the top? We can always add more as we go and move things around nothing is set in stone.

martinmicunda commented 10 years ago

sorry my mistake it should be like this:

time-booker/ 
  |- client/                
  |  |- src/   
  |  |  |- app/                 
  |  |  |  |- adapters/
  |  |  |  |- components/
  |  |  |  |- controllers/
  |  |  |- assets/
  |  |  |- vendor/
  |  |  |- index/

We should keep assets and vendor away from app logic so this structure is really clean.

by the way when u pull my code run this commands:

$ npm install ----> will install npm dependencies $ gulp install ----> will install bower dependencies and inject them into index.html

I will write documentation 2mor as I still try get test running and also heroku

chrislaughlin commented 10 years ago

Ah okay I have been just merging via the git hub site. Some questions ? Assets will be the third party libs like jquery or emberjs ? what will be in vendor?

Im happy with that layout

madole commented 10 years ago

Surely index.html should be in the app directory, makes no sense to have it one level above.

We vendor should only be anywhere near the code when it all gets built into a build directory, otherwise, it shouldn't be visible. Bower should download them at the point of building and chuck them into the build directory.

chrislaughlin commented 10 years ago

Also we should not have to run any commands when accepting pull requests it all should be in the commit.

Index should be in the app folder +1 Andy on that

madole commented 10 years ago

When we have the code built and a build directory with all the templates merged into the index.html etc.. then we can use gulp watch to live update the build directory as we are working on it

chrislaughlin commented 10 years ago

time-booker/ |- client/
| |- src/
| | |- app/
| | | |- adapters/ | | | |- components/ | | | |- controllers/ | | | |- index.html | | |- assets (all bower components)

madole commented 10 years ago

to hip chat

martinmicunda commented 10 years ago

assets ---> app images, fonts, css vendor ---> 3rd party lib installed by bower, this folder is never pushed into repo

index.html can't be in app folder because in express is pointing to static folder (in this case client/src)... Server should not care about where index page is.. you just need to specify where client code is ..

martinmicunda commented 10 years ago

Andrew I have reviewed your document and I think we want more/less same thing. I don't thing we should have styes, images, vendor under app directory that will get messy.. We should keep assets and vendor out of app folder. So we should stay we structure that we have at the moment as it really clean

Development:

time-booker/ 
  |- client/                --> all client code
  |  |- src/                    --> client source code files
  |  |  |- app/                     --> ember.js app code
  |  |  |- assets/                  --> static files like fonts, images, fonts
  |  |  |  |- fonts/                    --> fonts
  |  |  |  |- images/                   --> image files
  |  |  |  |- styles/                   --> css & sass files 
  |  |  |- vendor/                  --> 3rd party client and test libraries (installed by bower) **this folder is not store in github**
  |  |  |- index.html               --> app main file
  |  |  |- favicon.ico              --> favicon icon
  |  |- test/                   --> client test code
  |  |  |- config/                  --> configuration files for unit (karma) and e2e (protactor) tests
  |  |  |- e2e/                     --> e2e test files
  |  |  |- unit/                    --> unit test files
  |- server/                --> all server code   
  |  |- src/                    --> server source code files
  |  |- test/                   --> server test code
  |     |- config/                  --> configuration files for unit (karma) test
  |     |- unit/                    --> unit test files

Build (this is production build!):

build/               
  |- dist                       --> distribution source code that goes to production
  |  |- client/                     --> client code
  |  |  |- assets/                    --> emberJS-agnostic presentation artifacts 
  |  |  |  |- fonts/                     --> fonts
  |  |  |  |- images/                    --> image files
  |  |  |  |- styles/                    --> css files 
  |  |  |     |- app.min-12345.css          --> concat & minify app css files 
  |  |  |     |- lib.min-12345.css          --> concat & minify 3rd party lib css files 
  |  |  |- scripts/                   --> js files 
  |  |  |  |- app.min-12345.js           --> concat, minify angular app js files and cached html templates
  |  |  |  |- lib.min-12345.js           --> concat & minify 3rd party lib js files 
  |  |  |- index.html                 --> app main file
  |  |- server/                     --> server code 
  |- docs/                      --> app documentation    
  |- test-reports/              --> app test-reports (coverage, failure screenshots etc.)

Regarding to gulp tasks:

We don't need to create any dev task as I have already created template gulp task. The template task is used in gulp watch and gulp default tasks so it's compile templates to client/src/tmp/scripts/ember-templates.js and this file is already injected in index.html

NOTE: we probably gonna use same logic for css when we start use stylus or sass

I have also add develop which start express app and it restart server each time you make any change on server site.

So basically the dev flow should be follow:

Run the express app and watch for server changes:

   $ gulp develop

Install the bower dependencies and inject them to index.html and watch for client changes:

   $ gulp default

The reason why we gonna run two separate watches is that I don't want to restart app if I make a client changes. If I make a client change then only browser should be refreshed without to restart app.

Pre-commit git hook: We are using pre-commit git hook so on every commit we gonna run csslint, htmlhint, jshint , david and unit test. At the moment you when you commit you can see some erors but it doesn't stop you to commit because I have commented out the exit 1 in gulp that will doesn't let you commit if there is any jhsint, unit etc. error. I will put 'exit 1' statement back when clean code little bit and start follow all best practice.

TraviCI: TravisCI is already running and will run test against production code that is store in build/dist/ directory this code will be deploy to heroku by TravicCI if all tests will pass (I am currently working on heroku part). All travis artifact after each build will be store in [gh-pages] branch.

NOTE: i will commit travisCI code today

Once again it would be good when you could read the README.md file and play with gulp tasks.

chrislaughlin commented 10 years ago

I agree with martins approach it's easy to follow and not too much over kill I don't want us getting bogged down with processes and enjoy writing code

madole commented 10 years ago

Wow you guys have been busy this weekend. I've not updated my fork yet but I'll do that tonight and play with gulp.

Just a question, does gulp default minify the js code? For development, it's better not to minify the code in the build folder so it can be debugged. This question is probably irrelevant when i start to play with gulp but just to clarify.

chrislaughlin commented 10 years ago

Not sure but I don't think so, everything runs grand a heads up run npm install with sudo as it failed for me without it when installing proator

On Sunday, May 25, 2014, madole notifications@github.com wrote:

Wow you guys have been busy this weekend. I've not updated my fork yet but I'll do that tonight and play with gulp.

Just a question, does gulp default minify the js code? For development, it's better not to minify the code in the build folder so it can be debugged. This question is probably irrelevant when i start to play with gulp but just to clarify.

— Reply to this email directly or view it on GitHubhttps://github.com/cam-technologies/time-booker/pull/9#issuecomment-44129353 .

Regards

Chris Laughlin

madole commented 10 years ago

Yeah, I always sit in su when developing. Too much faffing around otherwise.. but then things like bower need flags like --allow-root

chrislaughlin commented 10 years ago

We shoul probably add that to the read me. I'm planning on adding some test to get near full coverage on what we have so far the. We can start proper tdd

On Sunday, May 25, 2014, madole notifications@github.com wrote:

Yeah, I always sit in su when developing. Too much faffing around otherwise.. but then things like bower need flags like --allow-root

— Reply to this email directly or view it on GitHubhttps://github.com/cam-technologies/time-booker/pull/9#issuecomment-44129413 .

Regards

Chris Laughlin

madole commented 10 years ago

Am I missing something here? Gulp default doesn't build the code into a build directory for me.

chrislaughlin commented 10 years ago

It just complies the templates into a file that's linked on the index.html. Where does the production task build the code

On Sunday, May 25, 2014, madole notifications@github.com wrote:

Am I missing something here? Gulp default doesn't build the code into a build directory for me.

— Reply to this email directly or view it on GitHubhttps://github.com/cam-technologies/time-booker/pull/9#issuecomment-44129671 .

Regards

Chris Laughlin

martinmicunda commented 10 years ago

Build directory is only for production it has nothing to do with development.

madole commented 10 years ago

Build directory should be building for development as well except that its unminified.

In this case, our source code version of index.html will have things that gulp has put in there, how do you then check changes into the index.html but exclude the auto generated code?

madole commented 10 years ago

my bad, i thought the following code was auto generated:

    <!-- bower:js -->
    <script src="vendor/jquery/dist/jquery.js"></script>
    <script src="vendor/handlebars/handlebars.js"></script>
    <script src="vendor/ember/ember.js"></script>
    <script src="vendor/bootstrap/dist/js/bootstrap.js"></script>
    <!-- endbower -->
    <!-- endbuild -->

is it?

martinmicunda commented 10 years ago

What do you mean by exclude the auto generated code?

martinmicunda commented 10 years ago

Yeah the blocks are the magic that do all work for us when we try build production code :)

chrislaughlin commented 10 years ago

Okay I can see what you mean so the references will be the same, for dev when we build the code can we have it still broken up into the different files and then for production all cocatted into one file

On Sunday, May 25, 2014, madole notifications@github.com wrote:

Build directory should be building for development as well except that its unminified.

In this case, our source code version of index.html will have things that gulp has put in there, how do you then check changes into the index.html but exclude the auto generated code?

— Reply to this email directly or view it on GitHubhttps://github.com/cam-technologies/time-booker/pull/9#issuecomment-44129792 .

Regards

Chris Laughlin

martinmicunda commented 10 years ago

By the way when u remove script and CSS link between bower blocks and u will run gulp install u can see how they are nicely injected back

chrislaughlin commented 10 years ago

Martin let's keep discussions on hip chat

On Sunday, May 25, 2014, Martin Micunda notifications@github.com wrote:

By the way when u remove script and CSS link between bower blocks and u will run gulp install u can see how they are nicely injected back

— Reply to this email directly or view it on GitHubhttps://github.com/cam-technologies/time-booker/pull/9#issuecomment-44129974 .

Regards

Chris Laughlin