Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove is_default Boolean from prices (rebased) #1469

Merged

Conversation

mamhoff
Copy link
Contributor

@mamhoff mamhoff commented Sep 24, 2016

This boolean is either a very bare-bones version of acts_as_paranoid or
a very bare-bones version of papertrail. However: It's really badly named,
as is_default is not something I understand very well at all. It probably means
"the price to be used in the backend and mostly in the frontend too".

We can accomplish the same behavior by simply using an order clause in
the currently_valid scope on prices.

This PR is #1182 rebased.

@mamhoff mamhoff force-pushed the remove-is-default-boolean-from-prices branch from e657822 to 5d5a158 Compare September 24, 2016 12:09
@jordan-brough
Copy link
Contributor

Seems reasonable but the specs are unhappy.

@mamhoff mamhoff force-pushed the remove-is-default-boolean-from-prices branch from 5d5a158 to 4dcc0a2 Compare September 24, 2016 15:25
This boolean is either a very bare-bones version of `acts_as_paranoid` or
a very bare-bones version of `papertrail`. However: It's really badly named,
as `is_default` is not something I understand very well at all.

We can accomplish the same behavior by simply using an `order` clause in
the `currently_valid` scope on prices.
@mamhoff mamhoff force-pushed the remove-is-default-boolean-from-prices branch from 4dcc0a2 to b2eed20 Compare September 24, 2016 15:39
@jhawthorn
Copy link
Contributor

👍

@jhawthorn jhawthorn merged commit acd46ed into solidusio:master Sep 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants