Apply changes required for ruby v2.5 #9
Conversation
| @@ -122,6 +122,7 @@ def serialization_column | |||
| end | |||
|
|
|||
| def remove_previous(before=nil, after=nil) | |||
| after ||= [] | |||
There was a problem hiding this comment.
Πως γινεται αυτο να δουλευε ως τωρα;;;!!!
Κοιτώντας τα δυο σημεία που χρησιμοποιούμε αυτη τη μεθοδο
https://github.com/skroutz/carrierwave/blob/skroutz/lib/carrierwave/mount.rb#L191
https://github.com/skroutz/carrierwave/blob/skroutz/lib/carrierwave/mount.rb#L341
θα προτεινα:
- Να φυγει το default nil απ το before
- Να μπει το default empty array στο after, στην παραμετρο, οχι μετα
There was a problem hiding this comment.
Δεν είμαι σίγουρος πως δούλευε χωρίς αυτό για να είμαι ειλικρηνής...
Ο λόγος που το έγραψα έτσι είναι επειδή στο πρωτότυπο repo με αυτο το συντακτικό προστέθηκε: https://github.com/carrierwaveuploader/carrierwave/blob/v1.1.0/lib/carrierwave/mounter.rb#L126
και με αυτο το συντακτικό παραμένει μέχρι και σήμερα:
https://github.com/carrierwaveuploader/carrierwave/blob/v2.1.0/lib/carrierwave/mounter.rb#L134
There was a problem hiding this comment.
Good catch. O λόγος που δεν μας έχει σκάσει στη μάπα είναι γιατί χρησιμοποιούμε την mount_uploader και όχι τη mount_uploaders. Η 1η περνάει πάντα, by default, arrays.
There was a problem hiding this comment.
Επίσης, προτείνω να το γράψουμε ακριβώς όπως το έχει ο upstream για καλύτερα rebase.
|
\cc @avgerin0s |
|
@strapro It'll be really helpful to enhance the commit message in order to describe, in bullets, the changes that we needed for Ruby 2.5. Both to facilitate the current review and to be able to trace it back in the future. |
Detailed list of changes: - features/support/activerecord.rb - Migrations are versioned in Rails 5 Copied from carrierwaveuploader/carrierwave commit 8c7dd59 - lib/carrierwave/mounter.rb - Rails 5.1.0 saved_changes behaves differently to changes: it returns the previous non existent change for the images column as nil instead of an empty array. Copied from carrierwaveuploader/carrierwave commit 053537d - spec/orm/activerecord_spec.rb - Fix pending spec not working in rspec 3.8.0 Copied from carrierwaveuploader/carrierwave commit ac5dd2e - Calling to_json on json attribute assignment is redundant and causing problem in Rails 5 Copied from carrierwaveuploader/carrierwave commit 30c6dfd - spec/support/activerecord.rb - Remove deprecated opt-in flag from ActiveRecord 5 Copied from carrierwaveuploader/carrierwave commit ddff3f0 - Gemfile - Add activemodel-serializers-xml to Rails 5 builds. Without it we get undefined method `to_xml' for Event (which is actually of type ActiveRecord::Base) Copied from carrierwaveuploader/carrierwave commit 1650051
strapro
force-pushed
the
skroutz-ruby-v2.5
branch
from
edfafc5 to
175ab4e
Compare
|
Enhanced the commit message with a detailed list of changes as well as the commit ids of the corresponding changes in the original carrierwaveuploader/carrierwave repository |
Changes required to get the rspec and cucumber tests to run successfully.