Skip to content
This repository was archived by the owner on Jul 19, 2025. It is now read-only.

Services Type Coercion#89

Merged
wfleming merged 4 commits intomasterfrom
will/bool-coercion
Mar 18, 2016
Merged

Services Type Coercion#89
wfleming merged 4 commits intomasterfrom
will/bool-coercion

Conversation

@wfleming
Copy link
Contributor

Virtus depends on the type of attributes being the
appopriate Axiom class. This led to incorrect & unexpected results in
app, where other classes with the same name were getting resolved,
causing quiet failures.

I rolled back yesterday's temporary patch & added some small sanity tests as well.

cc @codeclimate/review @pbrisbin

This reverts commit 490b779.

This was a temporary patch, and shouldn't be needed.
@@ -1,4 +1,4 @@
class CC::Service::Config
include Virtus.model
include Virtus.model(coerce: true)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd probably ditch this since it's the default and wasn't the issue.

Virtus pretty much depends on the type of attributes being the
appopriate `Axiom` class. This led to incorrect & unexpected results in
app, where other classes with the same name were getting resolved,
causing quiet failures.
This is the default.
@wfleming wfleming force-pushed the will/bool-coercion branch from aa5b542 to 66d2147 Compare March 18, 2016 17:05
@wfleming
Copy link
Contributor Author

@pbrisbin @gordondiggs Updated.

@pbrisbin
Copy link
Contributor

LGTM

wfleming added a commit that referenced this pull request Mar 18, 2016
@wfleming wfleming merged commit 949176b into master Mar 18, 2016
@wfleming wfleming deleted the will/bool-coercion branch March 18, 2016 18:55
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.

3 participants