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

Migration to Dragonfly ~> 1.0.0. #2500

Merged
merged 1 commit into from
Feb 1, 2014
Merged

Conversation

simi
Copy link
Member

@simi simi commented Dec 13, 2013

What wasn't tested:

  • S3 integration
  • acts_as_indexed integration

pingie @ugisozols @parndt

self.dragonfly_url_format = '/system/images/:job/:basename.:ext'
self.dragonfly_url_host = ''
self.trust_file_extensions = false
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe deprecation will be better to avoid error raising?

Copy link
Member

Choose a reason for hiding this comment

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

This is 3.0 so it's not that much of a problem. Everyone who is using this is on the edge, so... it depends.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's pain to migrate every refinery version to another refinery version. Let's start keeping it less painful and more friendly.

Copy link
Member

Choose a reason for hiding this comment

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

OK, that sounds good. So, Refinery.deprecate it is?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup. I'll change that later. 😴

@simi
Copy link
Member Author

simi commented Dec 14, 2013

I made few updates. I left it in separeted commit for easy review. I'll fixup this before merging.

@parndt
Copy link
Member

parndt commented Jan 8, 2014

Is this ready to go, in your opinion?

@simi
Copy link
Member Author

simi commented Jan 8, 2014

Not yet. I need to test it.

@simi
Copy link
Member Author

simi commented Jan 8, 2014

@parndt I'm getting some issues with refinery-acts-as-indexed master. Bundler can't resolve deps. :( So I can't test it.

@parndt
Copy link
Member

parndt commented Jan 8, 2014

@simi I'll take a look

@simi
Copy link
Member Author

simi commented Jan 8, 2014

☕ for you.

@parndt
Copy link
Member

parndt commented Jan 8, 2014

I get no issues with bundling your fork/branch, but maybe try a) rebasing this pull request against the current refinery/refinerycms and b) get the latest commits to refinery/refinerycms-acts-as-indexed@aa7862f then run bundle update :)

@simi
Copy link
Member Author

simi commented Jan 8, 2014

Shame on me. I was using old refinery/refinerycms-acts-as-indexed 👎

@simi
Copy link
Member Author

simi commented Jan 8, 2014

Hmm, are getting this also? I pointed to this branch (both, testing and refinerycms)

[retro@localhost  refinerycms-acts-as-indexed (master *=)]❤ bundle exec rake refinery:testing:dummy_app
rake aborted!
undefined method `load_tasks' for #<Refinery::Testing::Railtie:0x00000001d47d70>
/home/retro/.rvm/gems/ruby-2.0.0-p353/gems/railties-4.0.2/lib/rails/railtie/configurable.rb:30:in `method_missing'
/home/retro/work/fun/refinerycms-acts-as-indexed/Rakefile:16:in `<top (required)>'
(See full trace by running task with --trace)
diff --git a/Gemfile b/Gemfile
index 71beeb5..6713051 100644
--- a/Gemfile
+++ b/Gemfile
@@ -2,13 +2,12 @@ source 'https://rubygems.org'

 gemspec

-git 'git://github.com/refinery/refinerycms.git', :branch => 'master' do
-  gem 'refinerycms'
+gem 'refinerycms', path: '../refinerycms'

-  group :development, :test do
-    gem 'refinerycms-testing'
-  end
+group :development, :test do
+  gem 'refinerycms-testing', path: '../refinerycms'
 end
+
 gem 'refinerycms-i18n', github: 'refinery/refinerycms-i18n', branch: 'master'
 gem 'mime-types', '~> 1.16'

@parndt
Copy link
Member

parndt commented Jan 8, 2014

Yeah, that's due to old code. I'm currently fixing this, or trying!

@parndt
Copy link
Member

parndt commented Jan 8, 2014

@simi
Copy link
Member Author

simi commented Jan 8, 2014

OK, let's go thru CI and I'll rebase into one commit. It will be ok to merge in master, but please don't release yet. I tested acts-as-indexed, so it works as before.

@simi
Copy link
Member Author

simi commented Jan 8, 2014

@parndt 🍏

@simi
Copy link
Member Author

simi commented Jan 8, 2014

Failing test fixed here maybe - #2512. @parndt

@parndt
Copy link
Member

parndt commented Jan 15, 2014

@simi I merged your CI pull request

@simi
Copy link
Member Author

simi commented Jan 21, 2014

@parndt so?

@parndt
Copy link
Member

parndt commented Jan 21, 2014

@simi I'm happy but the PR is saying it cannot be automatically merged. Can you rebase the branch please?

@simi
Copy link
Member Author

simi commented Jan 22, 2014

tumblr_m9so4yoqgy1revsmeo1_500

parndt added a commit that referenced this pull request Feb 1, 2014
Migration to Dragonfly ~> 1.0.0.
@parndt parndt merged commit ff83c8f into refinery:master Feb 1, 2014
@parndt
Copy link
Member

parndt commented Feb 1, 2014

Thank you!!!!!!

@simi
Copy link
Member Author

simi commented Feb 1, 2014

tumblr_lx5fwjato61qmijb2o1_500

@simi simi deleted the draconfly-1 branch February 1, 2014 12:01
@parndt
Copy link
Member

parndt commented Feb 1, 2014

high-six

@simi simi mentioned this pull request Dec 6, 2014
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.

3 participants