mediasuitenz / mappy

A configurable map engine
MIT License
3 stars 2 forks source link

Pulls/key templating #59

Closed benmanu closed 9 years ago

benmanu commented 9 years ago

Changing key templating to use config defined handlebars templates, and exposing the key on the mappy object.

digitalsadhu commented 9 years ago

@benmanu @flashbackzoo I like the ability to define custom key templating but I think it should probably be optional.

Perhaps keep the loading of the existing template and then check if the user has defined the correct keys in config and use those if so.

Update: Also, I think some documentation around how the templating properties should be defined and a test or 2 would be great.

benmanu commented 9 years ago

@digitalsadhu I've done some further work around setting a default template, and adding some more details to the readme for the key config. Just getting my head around the testing side of it so will push that through once I have it up and running smoothly.

digitalsadhu commented 9 years ago

@benmanu nice! Thanks man

benmanu commented 9 years ago

@digitalsadhu I've fixed up the broken key tests and added a custom template test to key.spec. If you could check the coverage is sufficient that would be awesome

digitalsadhu commented 9 years ago

@benmanu Would you mind rebasing the latest changes from the main repo onto you branch and re-pushing? It's been a while since the original PR came in and its gotten quite far out of date in the previous days merge chaos.

Not sure what your git practices are but we always rebase latest master onto a feature branch before merging it onto master so that we get a nice clean readable history. This means we always have to force push our branches back up after rebase which some people don't like...

benmanu commented 9 years ago

@digitalsadhu I've managed to rebase the changes onto master, there wasn't too many conflicts, a few test fixes but looking all good.

digitalsadhu commented 9 years ago

Awesome @benmanu will merge in tonight

digitalsadhu commented 9 years ago

Fixed in #87 Had to do a final rebase to account for some changes made earlier today