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

Issue 1867 - images_controller.rb #1914

Closed
wants to merge 2 commits into from
Closed

Issue 1867 - images_controller.rb #1914

wants to merge 2 commits into from

Conversation

tbuyle
Copy link
Contributor

@tbuyle tbuyle commented Aug 29, 2012

See #1867

@travisbot
Copy link

This pull request passes (merged 9e2658c into c8a01a4).

@ugisozols
Copy link
Member

I can't add inline comments (probably because of the repo switch).

@tbuyle maybe we could shorten the code to:

@images << (@image = ::Refinery::Image.create({:image => image}.merge(params[:image].except(:image))))

Note the except(:image) part. WDYT?

@tbuyle
Copy link
Contributor Author

tbuyle commented Sep 5, 2012

Looks great : Less code, same result ;-D

I simply did not know about except in rails - Loos like it handles the dup and delete all by itself. Thanks !

@ugisozols
Copy link
Member

Can you rebase your PR with the suggested change and I'll merge it. Thanks!

@travisbot
Copy link

This pull request passes (merged ed4a200 into c8a01a4).

@ugisozols
Copy link
Member

Merged in b900c61. Thanks @tbuyle!

@ugisozols ugisozols closed this Sep 5, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tbuyle @travisbot @ugisozols