cds-snc / node-starter-app

Quick start application setup.... because you have to start somewhere.
MIT License
5 stars 3 forks source link

Cleanup macros #134

Open CalvinRodo opened 4 years ago

CalvinRodo commented 4 years ago

Did some light cleanup work in the macros to remove some copy and pasted sections and pull them into their own macros should improve maintainability.

I also moved macros that are only included in other macros into sub-macros folder (I can't think of a better name right now).

These could also be mashed up into a single file and import from there instead of having 3 imports in each file. However I've got some old baggage from my C# days about having only one thing per file.

timarney commented 4 years ago

First off I like this idea as it cleans things up in general and makes it easier to modify from a single spot.

@jneen do you see any issues getting this merged into the work your doing in https://github.com/cds-snc/node-starter-app/pull/132? I'm not spotting anything major at given a quick look.

CalvinRodo commented 4 years ago

Changelog has been updated

timarney commented 4 years ago

@CalvinRodo feel free to merge.

jneen commented 4 years ago

Oy, okay, just seeing this now. It's going to be a bit of work to integrate this.

timarney commented 4 years ago

Holding for @jneen to review.

jneen commented 4 years ago

I think that might be best - this seems easier to rebase against my changes. The only major thing that would have to change is the validationMessage macro, which currently seems to ignore its msg parameter. It should take just the relname parameter and use getError.