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

Migrate from Paperclip (EOL) to Active Storage #1114

Merged
merged 8 commits into from
May 30, 2023
Merged

Migrate from Paperclip (EOL) to Active Storage #1114

merged 8 commits into from
May 30, 2023

Conversation

johnbumgardner
Copy link
Member

@johnbumgardner johnbumgardner commented May 27, 2023 •
edited by CloCkWeRX
Loading

Fixes #841

Migrate from Paperclip to Active Storage.

Guidance to users:

While every effort has been made to migrate data

@johnbumgardner johnbumgardner requested a review from CloCkWeRX May 27, 2023 00:12
@johnbumgardner johnbumgardner changed the title make migration of paperclip to active storage issue #841 Migrate from Paperclip (EOL) to Active Storage May 27, 2023
@johnbumgardner johnbumgardner linked an issue May 27, 2023 that may be closed by this pull request
Migrate from Paperclip (EOL'd) to ActiveStorage or similar #841
Closed
github-advanced-security[bot]
github-advanced-security bot found potential problems May 27, 2023
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Rubocop found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

@johnbumgardner
Copy link
Member Author

Might need some help with some of the different schema.rb changes. Bit rusty on my Ruby on Rails

github-advanced-security[bot]
github-advanced-security bot found potential problems May 27, 2023
View reviewed changes
db/migrate/20230526212613_convert_to_active_storage.rb
class ConvertToActiveStorage < ActiveRecord::Migration[5.2]
require 'open-uri'

def up

Check notice

Code scanning / Rubocop

Avoid methods longer than 10 lines of code.

Metrics/MethodLength: Method has too many lines. [53/36]
@CloCkWeRX
Copy link
Member

So, from a code point of view I'm pretty happy with that.

For a changelog / upgrading instructions stance, could you adjust the PR summary to have a bit more detail on what users should do; to ensure there's no data lost?

This will warrant a major version bump I think.

@CloCkWeRX CloCkWeRX merged commit 3adf69d into master May 30, 2023
@johnbumgardner
Copy link
Member Author

So, from a code point of view I'm pretty happy with that.

For a changelog / upgrading instructions stance, could you adjust the PR summary to have a bit more detail on what users should do; to ensure there's no data lost?

This will warrant a major version bump I think.

yeah will try to work on this this week

@CloCkWeRX CloCkWeRX deleted the migrate-from-paper-clip-to-active-storage branch May 31, 2023 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haiderRizvii haiderRizvii haiderRizvii approved these changes

@CloCkWeRX CloCkWeRX CloCkWeRX approved these changes

@johnnyshields johnnyshields Awaiting requested review from johnnyshields

@steveyken steveyken Awaiting requested review from steveyken

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

Successfully merging this pull request may close these issues.

Migrate from Paperclip (EOL'd) to ActiveStorage or similar
3 participants
@johnbumgardner @CloCkWeRX @haiderRizvii