Skip to content

Conversation

@mjankowski
Copy link
Contributor

@mjankowski mjankowski commented Dec 25, 2024

Resolves #103

@mjankowski
Copy link
Contributor Author

Looks like a quoting change between 3.3 and 3.4 leads to change in output, which causes one failure. Specifically:

This is generated under 3.3 and earlier...

      # ./spec/spec_spec.rb:2:in `block (2 levels) in <top (required)>'

But under 3.4...

      # ./spec/spec_spec.rb:2:in 'block (2 levels) in <top (required)>'

(Note the change in ` to ' there)

Not sure how critical that format aspect is, or full context on that spec.

@jgmontoya
Copy link
Contributor

Hey @mjankowski! I made an attempt to fix the failing test, feel free to cherrypick my commit a7ebab0 here

@mjankowski
Copy link
Contributor Author

Thanks -- going to stand by on maintainer feedback re: desired path here, but will grab your commit if we're fine w/ that level of precision (again I dont fully understand context here). Seems like it more or less preserves intentions while being more flexible on apparently-version-specific output format.

@jgmontoya
Copy link
Contributor

Hey @briandunn!! First of all, thanks for maintaining this gem, it's awesome!

Are you ok with the approach taken here?
The 'issue' is regarding this change to Ruby:

Use a single quote instead of a backtick as a opening quote. [Feature #16495]

ruby/ruby#9608

@mjankowski
Copy link
Contributor Author

Oh yeah, also - separately from this formatting issue ... I can confirm that actually using/running flatware on 3.4.1 (with this PR branch allowing bundle) appears to be totally fine! My suite runs are as parallel as they've ever been.

@jgmontoya
Copy link
Contributor

👋 Two things:

  1. Here's an alternative solution to the failing test: 4957ef9 just to have an additional option available
  2. I've also verified that flatware works with rspec and ruby 3.4.1 using this branch (commit 7f03671)

@mjankowski
Copy link
Contributor Author

Updated to add one of those commits which also fixes the failure. Should be passing now.

@briandunn whenever you are back from holiday let me know if this makes sense or you'd prefer some other approach.

Copy link
Owner

@briandunn briandunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much! Loosening the assertion was exactly what I had in mind when I saw the initial failures. nice.

@briandunn briandunn merged commit 1931edc into briandunn:master Jan 3, 2025
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow ruby 3.4?

3 participants