Restructuring the application layout

esbtools / esb-message-admin

GNU General Public License v3.0

5 stars 9 forks source link

Restructuring the application layout #148

Closed bdmiller3 closed 8 years ago

bdmiller3 commented 8 years ago

Fixes #64

alechenninger commented 8 years ago

At a glance, looking good. I will say we should standardize on 2-space tabs for JavaScript. That is basically industry standard for JS, and it's much nicer to read IMHO.

alechenninger commented 8 years ago

I'll do a more thorough review after break.

alechenninger commented 8 years ago

Overall, looks good. Here's what I'd change:

Use 2 space tabs
Create a separate folder for synckeys view instead of putting the tpl and js in sync/
Make tests mirror app layout (so src/test/WEBAPP/app/[feature]/[tests] just so there's an obvious place to put tests also. (In doing this you'll need to update karma configs slightly)

alechenninger commented 8 years ago

Alright, next small request: in the same vein as the grid resize controller deal, let's move some of the stuff in shared to a more specific spot. I think shared should be for very generic things that are used in more than one place or have little to do with the app specifically. Like the message service makes sense there, and the local storage wrapper. I would move the rest of the files to error/ though, since they exist purely to support that tab (errcolprefs.svc.js, gridresize.ctrl.js, and nggrid.svc.js).

alechenninger commented 8 years ago

I think this is quite sane. Let's try and deploy it tomorrow and see if we missed anything.

alechenninger commented 8 years ago

Thanks!