Ruby 2.7 compatibility #28

janko · 2019-12-27T15:47:24Z

In Ruby 2.7 Nil#to_s now returns a frozen string, which caused FrozenError exceptions in CompositeIO#read when outbuf was nil. So, we update the code to handle this properly.

Ruby 2.7 also added deprecation warnings for the upcoming separation of positional and keyword arguments that is planned for Ruby 3.0. There were couple of specs that triggered the warning, so I updated the code to use the correct syntax.

I updated the Travis CI matrix to add 2.5, 2.6, and 2.7.

ixti · 2019-12-27T20:24:05Z

.travis.yml

@@ -19,13 +19,16 @@ rvm:
  # Include JRuby first because it takes the longest
  - jruby-9.1.13.0
  - 2.2
-  - 2.3.4
-  - 2.4.1
+  - 2.3


We don't need to support those anymore.

ixti

Thank you!

ixti · 2019-12-27T20:25:18Z

lib/http/form_data/composite_io.rb

-        outbuf = outbuf.to_s.clear
-        # buffer in JRuby is sometimes US-ASCII, force to ASCII-8BIT
-        outbuf.force_encoding(Encoding::BINARY)
+        data = outbuf ? outbuf.clear.force_encoding(Encoding::BINARY) : "".b


I feel neutral, but probably safenave here will look better:

Suggested change

data = outbuf ? outbuf.clear.force_encoding(Encoding::BINARY) : "".b

data = outbuf&.clear&.force_encoding(Encoding::BINARY) || "".b

Or two lines 😂:

Suggested change

data = outbuf ? outbuf.clear.force_encoding(Encoding::BINARY) : "".b

outbuf &&= outbuf.clear.force_encoding(Encoding::BINARY)

outbuf ||= "".b

Just to make it clear - I'm fine to keep the code the way it is now in the PR. Just throwing in ideas how to avoid ternary operator (I usually don't like it unless it's something super simple - like when branches are simple variables or literals).

Makes sense about avoiding the ternary operator. I want the data variable in order to make a distinction between outbuf being set and not, so how about this?

data = outbuf.clear.force_encoding(Encoding::BINARY) if outbuf data ||= "".b

Works for me 👍

.travis.yml

ixti · 2019-12-27T23:35:22Z

.rubocop.yml

+Metrics/CyclomaticComplexity:
+  Max: 7
+


Let me think a bit on how to avoid changing this.

ixti · 2019-12-27T23:35:34Z

.rubocop.yml

+
+Style/Next:
+  Enabled: false


Same about this one.

janko · 2020-01-04T19:17:32Z

@ixti I was able to fix both .rubocop.yml offenses 👍

janko · 2020-01-09T11:58:32Z

@ixti Any chance we could get this merged and released? Normally if it were only kwargs warnings it wouldn't be a big deal, but these are actual errors, so it would be useful to get the fix released 😃

Afterwards, we can bump http-form_data version on the http gem and get it released as well.

This would help a lot in getting Shrine updated for Ruby 2.7 🙏

I believe this option was changed in RubyGems 3.0.

These have reached EOL.

I don't think we need to specify Bundler at all in the gemspec, so we remove it.

As of Ruby 2.7, Nil#to_s returns a frozen string. We update the code to ensure a mutable string is returned for the output buffer.

See https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/

janko · 2020-01-09T12:10:13Z

I also cleaned up the commit history 😅

ixti

Please make supported list of rubies explicit, and then it will be fine. ))

ixti · 2020-01-09T12:25:17Z

README.md

-* Ruby 2.3.x
-* Ruby 2.4.x
-* JRuby 9.1.x.x
+* Ruby 2.4+


After recent incident in http gem I would like to keep this list explicit

Updated 👍

ixti

Looks awesome. Thank you very much.

ixti · 2020-01-09T13:24:49Z

I'll cut release shortly.

ixti · 2020-01-09T14:40:36Z

Released as v2.2.0

@summera

Update ruby-http-form_data to 2.3.0. pkgsrc change: add "USE_LANGAUGES= # none". ## 2.3.0 (2020-03-08) * [#29](httprb/form_data#29) Enhance HTTP::FormData::Urlencoded with per-instance encoder. [@summera][] ## 2.2.0 (2020-01-09) * [#28](httprb/form_data#28) Ruby 2.7 compatibility. [@janko][]

janko mentioned this pull request Dec 27, 2019

Ruby 2.7 compatibility httprb/http#581

Merged

ixti reviewed Dec 27, 2019

View reviewed changes

ixti approved these changes Dec 27, 2019

View reviewed changes

ixti reviewed Dec 27, 2019

View reviewed changes

.travis.yml Outdated Show resolved Hide resolved

ixti reviewed Dec 27, 2019

View reviewed changes

janko mentioned this pull request Jan 4, 2020

Ruby 2.7 compatibility shrinerb/shrine#437

Closed

janko added 6 commits January 9, 2020 13:02

Use --no-document for gem install on Travis

40bdaec

I believe this option was changed in RubyGems 3.0.

Drop support for MRI 2.2 and 2.3

4dd7cac

These have reached EOL.

Allow using Bundler 2

ef3ceab

I don't think we need to specify Bundler at all in the gemspec, so we remove it.

Add MRI 2.7 to the Travis matrix

05096b0

Fix FrozenError exception on Ruby 2.7

b005f6a

As of Ruby 2.7, Nil#to_s returns a frozen string. We update the code to ensure a mutable string is returned for the output buffer.

Fix kwargs warnings on Ruby 2.7

1095586

See https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/

janko force-pushed the ruby-2-7-compatibility branch from b3df1d6 to 1095586 Compare January 9, 2020 12:09

ixti approved these changes Jan 9, 2020

View reviewed changes

Keep using explicit supported versions in README

15f097e

ixti approved these changes Jan 9, 2020

View reviewed changes

ixti merged commit 54c8ba2 into httprb:master Jan 9, 2020

janko deleted the ruby-2-7-compatibility branch January 9, 2020 14:08

janko mentioned this pull request Jan 9, 2020

Bump http-form_data dependency to 2.2+ httprb/http#584

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ruby 2.7 compatibility #28

Ruby 2.7 compatibility #28

janko commented Dec 27, 2019 •

edited

Loading

ixti Dec 27, 2019

ixti left a comment

ixti Dec 27, 2019

ixti Dec 27, 2019

ixti Dec 27, 2019

janko Dec 27, 2019 •

edited

Loading

ixti Dec 27, 2019

ixti Dec 27, 2019

ixti Dec 27, 2019

janko commented Jan 4, 2020 •

edited

Loading

janko commented Jan 9, 2020 •

edited

Loading

janko commented Jan 9, 2020

ixti left a comment

ixti Jan 9, 2020

janko Jan 9, 2020

ixti left a comment

ixti commented Jan 9, 2020

ixti commented Jan 9, 2020

	data = outbuf ? outbuf.clear.force_encoding(Encoding::BINARY) : "".b
	data = outbuf&.clear&.force_encoding(Encoding::BINARY) \|\| "".b

	data = outbuf ? outbuf.clear.force_encoding(Encoding::BINARY) : "".b
	outbuf &&= outbuf.clear.force_encoding(Encoding::BINARY)
	outbuf \|\|= "".b

		Metrics/CyclomaticComplexity:
		Max: 7

Ruby 2.7 compatibility #28

Ruby 2.7 compatibility #28

Conversation

janko commented Dec 27, 2019 • edited Loading

Choose a reason for hiding this comment

ixti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janko Dec 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janko commented Jan 4, 2020 • edited Loading

janko commented Jan 9, 2020 • edited Loading

janko commented Jan 9, 2020

ixti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ixti left a comment

Choose a reason for hiding this comment

ixti commented Jan 9, 2020

ixti commented Jan 9, 2020

janko commented Dec 27, 2019 •

edited

Loading

janko Dec 27, 2019 •

edited

Loading

janko commented Jan 4, 2020 •

edited

Loading

janko commented Jan 9, 2020 •

edited

Loading