Add ability to disable Geddy models in configuration #479

Merged
merged 2 commits into from Oct 31, 2013

Conversation

Projects
None yet
2 participants
Contributor

kr1zmo commented Oct 31, 2013

This will allow model: false in geddy environment configuration
(config.js), which disables the importation of models on initiation.
Models are no longer defined in geddy.model['name'] and instead must be
required directly by controllers or logic that utilize them.

Issue #427

Add ability to disable Geddy models in configuration
This will allow model: false in geddy environment configuration
(config.js), which disables the importation of models on initiation.
Models are no longer defined in geddy.model['name'] and instead must be
required directly by controllers or logic that utilizes them.

mde commented on d3dee37 Oct 31, 2013

I think this is fine as a general approach. A couple of thoughts:

  • We already have the check right below that bails out if there's no 'model' directory in the app. Your check for the config value should probably just be another check in the same place.
  • The check you're doing is redundant -- if the value of config.model is triple-equal to false, it is by definition a Boolean. All you need is the second check.
Contributor

kr1zmo commented Oct 31, 2013

@mde
Right, completely, I'll combine it with the if statement below and reduce the complexity to loose a stupid redundant pattern.

Ability to disable Geddy models in configuration
This will allow model: false in geddy environment configuration
(config.js), which disables the importation of models on initiation.
Models are no longer defined in geddy.model['name'] and instead must be
required directly by controllers or logic that utilizes them.
Contributor

kr1zmo commented Oct 31, 2013

@mde, let me know if you need anything else or if you feel a different approach is necessary.

mde added a commit that referenced this pull request Oct 31, 2013

Merge pull request #479 from kr1zmo/master
Add ability to disable Geddy models in configuration

@mde mde merged commit 59fa16e into geddy:master Oct 31, 2013

1 check passed

default The Travis CI build passed
Details
Contributor

mde commented Oct 31, 2013

Looks good, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment