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

[gherkin perl] general cleanup by upgrading the Perl dependency #1665

Merged
merged 9 commits into from
Jul 21, 2021

Conversation

ehuelsmann
Copy link
Contributor

@ehuelsmann ehuelsmann commented Jul 18, 2021

Summary

Miscelaneous cleanup made possible after upgrading the Perl minimum requirement to 5.12.0 (which was released in April 2010).

Details

  • Since 5.12, Perl can open filehandles to in-memory byte sequences. Remove an external depenency (IO::Scalar) by taking advantage of that.
  • Additionally, since 5.12, all file handles are blessed references into IO::File, which removes the need to explicitly load and create IO::File instances.
  • Close the input file as soon as the last line is read, instead of when the TokenScanner goes out of scope; it's simply better form to do that early

Motivation and Context

The resulting code is overall more clean and more modern Perl.

How Has This Been Tested?

The test suite runs on this change as expected.

Types of changes

  • General cleanup.
  • Dependency update.

Checklist:

  • The change does not need to be ported to other implementations
  • Existing tests cover this change.
  • My change does not require a change to the documentation.
  • No documentation changes are required.
  • I have updated the CHANGELOG accordingly.

There's no need to close the file in the DESTROY method, because the
file is closed as soon as the file descriptor goes out of scope (which
is what triggers the DESTROY call too).
Note that the new `Encode` dependency is a core module, which means
it's a dependency, but it's not external. Therefore, it doesn't need
to be declared as such in the 'cpanfile'.
ehuelsmann and others added 5 commits July 18, 2021 13:44
The Perl ecosystem slowly moves from Test::More to Test2. The latter
is better architected and has many, many more options for state
validation out of the box.
Make sure the token scanner doesn't skip empty lines and strips
carriage returns as intended.
@aurelien-reeves
Copy link
Contributor

@ehuelsmann is this PR ready to be merged?

@ehuelsmann
Copy link
Contributor Author

@aurelien-reeves yes, it is. I was waiting for an answer to the effect on version numbering; @olleolleolle explained that upping the minimum version should be only a minor version upgrade. So, lets merge!

@ehuelsmann
Copy link
Contributor Author

Merging with the implied consent.

@ehuelsmann ehuelsmann merged commit 79f2b9a into main Jul 21, 2021
@ehuelsmann ehuelsmann deleted the gherkin-perl-modernized branch July 21, 2021 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants