cucumber-ruby-core icon indicating copy to clipboard operation
cucumber-ruby-core copied to clipboard

[Experiment] get something working with new Value base class.

Open botandrose opened this issue 2 years ago • 5 comments

Description

Refactor Test::Case to use a new base class of Value, which encapsulates the idea of an immutable value object instantiated by keywords. This is very similar to the new Data class included in Ruby 3.2: https://docs.ruby-lang.org/en/3.2/Data.html, but with the extra feature of optional parameters.

This has a number of benefits: it resolves a number of Rubocop violations, it replaces long positional arguments with keyword arguments, it removes boilerplate, and makes explicit that these objects are Value Objects. https://en.wikipedia.org/wiki/Value_object

This is just a proof-of-concept, but if this looks like a direction we want to go in, I'll continue this PR with the following:

  1. Document Value
  2. Write specs for Value
  3. Convert the other value objects in core, as well.
  4. Remove Rubocop exceptions that are no longer relevant after this.
  5. Follow-up PRs for gems depending on core to use the new kwargs syntax where necessary.

Thoughts?

Type of change

  • Refactoring (improvements to code design or tooling that don't change behaviour)
  • Breaking change (will cause existing functionality to not work as expected)

I guess its both? Refactoring from the outer cucumber standpoint, but breaking API change from the core gem standpoint.

  • [ ] Tests have been added for any changes to behaviour of the code
  • [x] New and existing tests are passing locally and on CI
  • [x] bundle exec rubocop reports no offenses
  • [ ] RDoc comments have been updated
  • [ ] CHANGELOG.md has been updated

botandrose avatar Oct 05 '23 13:10 botandrose

Some of this is quite non-standard e.t.c. - Is it worth maybe just holding off some of this stuff if it's gonna get this complex until ruby 3.2 is the minimum, there's 1000 other tech debt things to fix meanwhile.

luke-hill avatar Oct 09 '23 08:10 luke-hill

@luke-hill Seeing as this data structure is included in Ruby 3.2 -- and in core, not std lib -- I'd say its as standard Ruby as it gets, as standard as Hash or Set.

We're currently supporting Ruby 2.5+, released in 2017, nearly six years ago. This is an attempt to get the benefits described above without waiting six-ish years for cucumber to require 3.2+.

botandrose avatar Oct 09 '23 18:10 botandrose

@luke-hill Great! Thank you very much. I'll rebase and continue with the TODO items I listed in the description.

botandrose avatar Nov 22 '23 16:11 botandrose

Heya @botandrose

I've been working (And will be slowly), on a minor release to cucumber-ruby to use all the new work we've done in core and other associated gems.

Naturally spec failures are abundant, but I've been fixing things up a bit. I also did a patch fix to core (not yet released).

Work is here: https://github.com/cucumber/cucumber-ruby/pull/1751 - Not sure if it will impact anything. Don't think so.

luke-hill avatar Jan 29 '24 13:01 luke-hill

Hey @luke-hill thanks for the heads-up!

I'm really looking forward to that release. That line number matching regression has kept me stuck on cucumber v3, and so I'll finally be able to rejoin the rest of the world on a modern cucumber release!

Once I'm dogfooding modern cucumber again, I'll be more motivated to hack on it.

Regarding that, and this PR specifically, I've made some explorations expanding this data structure to the other value object classes. The jury is still out on whether or not I want to actually recommend this PR for merging, but I hope to get back to it again soon.

botandrose avatar Jan 31 '24 15:01 botandrose