Mange / roadie-rails

Making HTML emails comfortable for the Rails rockstars
MIT License
363 stars 65 forks source link

Version 1.2.1 should be released immediately #73

Closed marutosi closed 7 years ago

marutosi commented 7 years ago

Version 1.2.0 is installed accidentally in ruby < 2.2 without #72.

Mange commented 7 years ago

Is that a problem right now? I'm not using any post-2.2 features in the codebase, right?

marutosi commented 7 years ago

If you define gem in Gemfile:

gem "roadie-rails", :git => 'git://github.com/Mange/roadie-rails.git',
        :ref => '17e0859195154e55c'

"bundle install/update" got:

Bundler could not find compatible versions for gem "ruby":
  In Gemfile:
    ruby

    roadie-rails was resolved to 1.2.0, which depends on
      ruby (>= 2.2)

But in Gemfile:

gem "roadie-rails"

You can install roadie-rails 1.2.0 in ruby < 2.2. Is it very dangerous?

ahorek commented 7 years ago

Hi @Mange, there is one incompatibility on windows introduced by this commit https://github.com/Mange/roadie-rails/commit/ad745f34586a309921346c2628ccfa08dec4105a

gem install roadie-rails
/lib/ruby/site_ruby/2.1.0/rubygems/package.rb:388:in `symlink': symlink() function is unimplemented on this machine (NotImplementedError)
        from /lib/ruby/site_ruby/2.1.0/rubygems/package.rb:388:in `block (2 levels) in extract_tar_gz'
        from /lib/ruby/site_ruby/2.1.0/rubygems/package/tar_reader.rb:65:in `each'
        from /lib/ruby/site_ruby/2.1.0/rubygems/package.rb:365:in `block in extract_tar_gz'
        from /lib/ruby/site_ruby/2.1.0/rubygems/package.rb:459:in `block in open_tar_gz'
        from /lib/ruby/site_ruby/2.1.0/rubygems/package.rb:456:in `wrap'
        from /lib/ruby/site_ruby/2.1.0/rubygems/package.rb:456:in `open_tar_gz'

the problem is symlinks are supported only on Ruby 2.2+ and also on more recent version you need administratior rights to be able to use symlinks. It worked in the previous version 1.1.1.

However symlinks are used only in your specs, is there a reason why are they included in the gem file? You can keep it in the repository, but you should remove these files from gemspec. What do you think?

Mange commented 7 years ago

That does sound like a real problem. Thank you for explaining it to me. I'll take a look at this tomorrow morning!

Den tor 11 maj 2017 16:08ahorek notifications@github.com skrev:

Hi @mange, there is one incompatibility on windows introduced by this commit Mange/roadie-rails@ad745f3 https://github.com/Mange/roadie-rails/commit/ad745f34586a309921346c2628ccfa08dec4105a

gem install roadie-rails /lib/ruby/site_ruby/2.1.0/rubygems/package.rb:388:in symlink': symlink() function is unimplemented on this machine (NotImplementedError) from /lib/ruby/site_ruby/2.1.0/rubygems/package.rb:388:inblock (2 levels) in extract_tar_gz' from /lib/ruby/site_ruby/2.1.0/rubygems/package/tar_reader.rb:65:in each' from /lib/ruby/site_ruby/2.1.0/rubygems/package.rb:365:inblock in extract_tar_gz' from /lib/ruby/site_ruby/2.1.0/rubygems/package.rb:459:in block in open_tar_gz' from /lib/ruby/site_ruby/2.1.0/rubygems/package.rb:456:inwrap' from /lib/ruby/site_ruby/2.1.0/rubygems/package.rb:456:in `open_tar_gz'

the problem is symlinks are supported only on Ruby 2.2+ and also on more recent version you need administratior rights to be able to use symlinks. It worked in the previous version 1.1.1.

However symlinks are used only in your specs, is there a reason why are they included in the gem file? You can keep it in the repository, but you should remove these files from gemspec. What do you think?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/Mange/roadie-rails/issues/73#issuecomment-300800540, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAGP1JGFeon00EVwkv-3ypGjsUpBT1uks5r4xZKgaJpZM4NWj7K .

ahorek commented 7 years ago

if you agree with not including specs in a .gem file https://github.com/Mange/roadie-rails/pull/74 and there is no other incompatibility I think you can allow older versions of ruby even if they won't be officialy supported.

spec.required_ruby_version = ">= 2.2"
Mange commented 7 years ago

Version 1.2.1 has been released.

domcleal commented 7 years ago

If #74 was merged, why is setting the minimum Ruby version to 2.2 necessary? Wasn't #74 the fix for the symlink issue on Windows?

It's a surprise to see the minimum Ruby version change in a patch release, it looks unnecessary now.

Mange commented 7 years ago

It's because I don't technically support Ruby < 2.2 in the 1.2 branch, and I want to avoid accidentally breaking things for those users later IF I start to use Ruby 2.2 features like keyword arguments.

I was just questioning the urgency if this, not the change itself.

Den fre 12 maj 2017 09:22Dominic Cleal notifications@github.com skrev:

If #74 https://github.com/Mange/roadie-rails/pull/74 was merged, why is setting the minimum Ruby version to 2.2 necessary? Wasn't #74 https://github.com/Mange/roadie-rails/pull/74 the fix for the symlink issue on Windows?

It's a surprise to see the minimum Ruby version change in a patch release, it looks unnecessary now.

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/Mange/roadie-rails/issues/73#issuecomment-301003511, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAGP5-V1usym8WeD-hlf2iGZBDTOxaTks5r5AibgaJpZM4NWj7K .

domcleal commented 7 years ago

Ah thanks for the response, I'd missed that in the CHANGELOG.