relaton / relaton-bib

MIT License
3 stars 1 forks source link

Support Ruby 3 #38

Closed skalee closed 3 years ago

skalee commented 3 years ago

This pull request adds Ruby 3.0 compatibility, or at least makes tests passing locally. It's likely that other Relaton gems require changes too. I was experimenting with Ruby 3.0 compatibility on this gem only.

The only significant change is that double splat operator (**) is used when passing a hash to a method which expects keyword arguments rather than actual hash. This is required because Ruby 3.0 no longer decomposes such hashes implicitly (see https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/).

I am not sure if these are the only required changes, but they are enough to make tests passing.

Neither I'm sure if all these methods really should accept keyword arguments. Perhaps it'd be better to accept hash as a single positional argument. I wasn't analyzing that.

Apart from codebase itself, some dependencies also aren't compatible with Ruby 3.0 yet:

ronaldtse commented 3 years ago

@andrew2net @skalee this is also blocking metanorma-standoc: https://github.com/metanorma/metanorma-standoc/pull/459/checks?check_run_id=2377920739

ronaldtse commented 3 years ago

And we need a rebase please, thanks!

w00lf commented 3 years ago

Hi there, @skalee we have an issue with relaton: https://github.com/metanorma/metanorma-standoc/pull/459/checks?check_run_id=2377920739

Failure/Error: bibitemxml = RelatonBib::BibliographicItem.new(RelatonBib::HashConverter::hash_to_bib(bib)).to_xml or next

     ArgumentError:
       wrong number of arguments (given 1, expected 0)
     # ./vendor/bundle/ruby/3.0.0/gems/relaton-bib-1.7.4/lib/relaton_bib/bibliographic_item.rb:192:in `initialize'
andrew2net commented 3 years ago

@ronaldtse relaton fully supports Ruby 3.0 now. We don't need this PR anymore.

ronaldtse commented 3 years ago

Thanks @andrew2net @skalee , closing.

andrew2net commented 3 years ago

Hi there, @skalee we have an issue with relaton: https://github.com/metanorma/metanorma-standoc/pull/459/checks?check_run_id=2377920739

@w00lf the issue is in the ./lib/asciidoctor/standoc/cleanup_ref_dl.rb:11 line:

bibitemxml = RelatonBib::BibliographicItem.new(RelatonBib::HashConverter::hash_to_bib(bib)).to_xml or next

To work in Ruby 3.0 it should call RelatonBib::HashConverter::hash_to_bib(**bib)

skalee commented 3 years ago

I confirm that relaton-bib works in Ruby 3 now. Thanks!

w00lf commented 3 years ago

Hi there, @skalee we have an issue with relaton: https://github.com/metanorma/metanorma-standoc/pull/459/checks?check_run_id=2377920739

@w00lf the issue is in the ./lib/asciidoctor/standoc/cleanup_ref_dl.rb:11 line:

bibitemxml = RelatonBib::BibliographicItem.new(RelatonBib::HashConverter::hash_to_bib(bib)).to_xml or next

To work in Ruby 3.0 it should call RelatonBib::HashConverter::hash_to_bib(**bib)

Tried to apply your hotfix but encountered a situation when specs fail on ruby version less than 2.7 with error:

 TypeError:
       hash key "id" is not a Symbol

It seems that ruby version less than 2.7 does not support string keys, if i try to use RelatonBib::HashConverter::hash_to_bib(**bib.symbolize_keys) i got an error in ruby 3.0: wrong number of arguments (given 1, expected 0)

andrew2net commented 3 years ago

@w00lf sorry, it's my mistake. Try to replace the line

bibitemxml = RelatonBib::BibliographicItem.new(RelatonBib::HashConverter::hash_to_bib(bib)).to_xml or next

with this

bibitemxml = RelatonBib::BibliographicItem.from_hash(bib).to_xml
w00lf commented 3 years ago

@w00lf sorry, it's my mistake. Try to replace the line

bibitemxml = RelatonBib::BibliographicItem.new(RelatonBib::HashConverter::hash_to_bib(bib)).to_xml or next

with this

bibitemxml = RelatonBib::BibliographicItem.from_hash(bib).to_xml

That works, all tests are passing. Thanks!