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.

Sign up for GitHub

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

Jump to bottom

Remove belongs_to :return_authorization from InventoryUnit #2753

Merged
merged 1 commit into from
Jan 30, 2019
Merged

Remove belongs_to :return_authorization from InventoryUnit #2753

merged 1 commit into from
Jan 30, 2019

Conversation

snarfmason
Copy link

This association appears to do nothing, and specs all pass without it.

Remove belongs_to :return_authorization from InventoryUnit
98b1a75 
This association appears to do nothing, and specs all pass without it.
gmacdougall
gmacdougall approved these changes Jun 29, 2018
View reviewed changes
Copy link
Member

@gmacdougall gmacdougall left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! I agree this appears to do nothing, and is not the inverse of the thing it says it does.

spaghetticode
spaghetticode approved these changes Jan 28, 2019
View reviewed changes
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.

@snarfmason thanks for spotting this 👍

aitbw
aitbw approved these changes Jan 29, 2019
View reviewed changes
Copy link
Contributor

@aitbw aitbw left a comment

Choose a reason for hiding this comment

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

Thanks, @snarfmason ! 🙌

jacobherrington
jacobherrington approved these changes Jan 29, 2019
View reviewed changes
Copy link
Contributor

@jacobherrington jacobherrington left a comment

Choose a reason for hiding this comment

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

Thanks @snarfmason! 🍰

@jacobherrington
Copy link
Contributor

@snarfmason this looks great, but could you rebase? Tests have changed and I want to make sure they still look good. Thanks for the PR!

@jacobherrington
Copy link
Contributor

This passes locally after rebase, and the association really does look like it's doing nothing. Going to merge 👍 Thanks @snarfmason!

@jacobherrington jacobherrington merged commit 51eae53 into solidusio:master Jan 30, 2019
@snarfmason
Copy link
Author

@jacobherrington Apologize for missing this post. My github notifications have been a mess, I need to get that back under control. Thank you for rebasing that for me.

@snarfmason snarfmason deleted the remove-useless-association-from-inventory-unit branch February 12, 2019 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacobherrington jacobherrington jacobherrington approved these changes

@aitbw aitbw aitbw approved these changes

@gmacdougall gmacdougall gmacdougall approved these changes

@spaghetticode spaghetticode spaghetticode approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@snarfmason @jacobherrington @spaghetticode @gmacdougall @aitbw