inventid / tails

Models on the fly!
MIT License
1 stars 0 forks source link

Apply dynamic properties to attributes #18

Closed joostverdoorn closed 10 years ago

joostverdoorn commented 10 years ago

Dynamic properties haven't really proven useful, and they might make sense for model attributes. In this PR this change is also used in the relations mixin, where it simplifies the code.

Lazy properties could also be useful when we don't want to instantiate a property until it is needed. This might speed up loading times in some cases.

rogierslag commented 10 years ago

:+1: for code, but for completeness, could you rebase master onto this branch first and resolve the conflicts? Im curious if everything will still play nice together

And I want to see the Travis CI coverage data (which is not actually from travis but you get the idea)

joostverdoorn commented 10 years ago

I don't see the use of rebasing master onto this branch. What sort of conflicts would you expect?

rogierslag commented 10 years ago

Mostly since git apparently cant automerge this branch with develop (Github cant merge the PR). Rebasing beforehand gives us the possibility to run the test suite again and get insight if everything progresses as we hope

joostverdoorn commented 10 years ago

Ah, you mean develop, you said master. Sure, I'll rebase this branch onto develop. It updated since I created the PR. Git has issues merging the compiled versions (tails.js and tails.min.js) sometimes, so I'll just recompile them.

rogierslag commented 10 years ago

Plus I generally prefer that a Pull Request shows exactly what will happen with the merge, that way we can detect an incorrect merge before it goes into develop. In my opinion, a PR should be non-ambigious to what will happen when enough thumbs up are given

/edit: I mean develop, not master. Excuse me for that mistake

coveralls commented 10 years ago

Coverage Status

Coverage increased (+0.81%) when pulling b17b1d65da064ac1cbb8eadfb7f84e0a2d7ce807 on feature/apply-dynamic-properties-to-attributes into ae450c402deaec3416a42581d677bb5a04dc8b9d on develop.

rogierslag commented 10 years ago

:+1:

coveralls commented 10 years ago

Coverage Status

Coverage increased (+0.81%) when pulling 4e8b5a28d4ed8fd040e81e8ca7909f32be8674ee on feature/apply-dynamic-properties-to-attributes into ae450c402deaec3416a42581d677bb5a04dc8b9d on develop.

coveralls commented 10 years ago

Coverage Status

Coverage increased (+1.12%) when pulling 82e5048afeaf686b77e4affa0660d1b9ba449a8f on feature/apply-dynamic-properties-to-attributes into ae450c402deaec3416a42581d677bb5a04dc8b9d on develop.