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

Refactor Spree::Address value_attributes #3465

Conversation

filippoliverani
Copy link
Contributor

Description
Preparatory refactoring to simplify Spree::Address code and make easier working on #3234.

Checklist:

Copy link
Member

@spaghetticode spaghetticode left a comment

Choose a reason for hiding this comment

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

@filippoliverani looks good to me, thanks 👍

@filippoliverani filippoliverani force-pushed the refactor-address-value-attributes branch from 1e64f5c to 6a28a60 Compare December 23, 2019 08:38
Copy link
Member

@aldesantis aldesantis left a comment

Choose a reason for hiding this comment

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

@filippoliverani looking good, but can you remove the commit by dependabot?

@filippoliverani filippoliverani force-pushed the refactor-address-value-attributes branch from 6a28a60 to 8c6d93d Compare December 23, 2019 10:08
Preparatory refactoring to simplify code and make easier changing
value_attributes code in incoming PRs. Removing some conditionals and
in-place hash updates will make adding new `name` handling business
logic more straightforward.
@filippoliverani filippoliverani force-pushed the refactor-address-value-attributes branch from 8c6d93d to df6aa1a Compare December 23, 2019 10:10
@filippoliverani
Copy link
Contributor Author

@filippoliverani looking good, but can you remove the commit by dependabot?

@aldesantis Sorry, it slipped in during a rebase, removed 👍

@aldesantis aldesantis merged commit 479c7b7 into solidusio:master Jan 9, 2020
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