Skip to content
This repository has been archived by the owner on Feb 1, 2019. It is now read-only.

Refactor for mixins #115

Merged
merged 11 commits into from
Mar 27, 2015
Merged

Refactor for mixins #115

merged 11 commits into from
Mar 27, 2015

Conversation

LinuxBozo
Copy link
Contributor

Refactor the engine to make use of mixins. Refactor out globals as well and make use of getter/setters.

@LinuxBozo
Copy link
Contributor Author

For cfpb/hmda-pilot#305

@poorgeek
Copy link
Contributor

Any particular reason why the tests for lib/engineCustomConditions.js and lib/engineCustomDataLookupConditions.js aren't in their own specs?

var EngineCustomConditions = (function() {
return function() {

/* hmda-syntactical */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do comments like these actually add anything anymore? When everything was in single file they did help delineate between different areas, but now that you've modularized the code it seems like they should be replaced by jsdoc comments on each function. If so, that might be part of the system documentation task.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are just grouping/section type comments, so there still is value, even in the smaller components. Additional jsdoc comments would help, especially documentation tying each function to it's actual edit.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.68%) to 98.01% when pulling 60a2f5d on LinuxBozo:refactor into 223e720 on cfpb:milestone8.

@coveralls
Copy link

Coverage Status

Coverage increased (+2.2%) to 98.54% when pulling 6560781 on LinuxBozo:refactor into 223e720 on cfpb:milestone8.

@LinuxBozo
Copy link
Contributor Author

@poorgeek There ya go, tests broken out for the mixins..

@poorgeek
Copy link
Contributor

Cool. Looks good to me!

poorgeek added a commit that referenced this pull request Mar 27, 2015
@poorgeek poorgeek merged commit fc25109 into cfpb:milestone8 Mar 27, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants