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

Update to support Ruby 3 keyword args #228

Merged
merged 1 commit into from
May 29, 2022

Conversation

agirling
Copy link
Contributor

@geekq
Copy link
Owner

geekq commented May 23, 2022

Thanks for your contribution!
Backward compatibility is very important (people have used this library for many years, and some do not upgrade to the newest Ruby or Rails versions). The compatibility should be covered (checked) by the test suite though. Good thing would be to switch from travis-CI (ceased) to GitHub actions, before integrating any new contributions. Maybe something inspired by https://github.com/geekq/workflow-activerecord/blob/develop/.github/workflows/test.yml so any regression can be prevented.

For any new behavior we'll need test cases as well or is the PR just to make it work for both Ruby 2 and Ruby 3 without any new behavior?

@geekq
Copy link
Owner

geekq commented May 25, 2022

Blocked by gh-229

@geekq
Copy link
Owner

geekq commented May 26, 2022

Please rebase your PR, so we can see the results of the tests for different Ruby versions

@agirling
Copy link
Contributor Author

Please rebase your PR, so we can see the results of the tests for different Ruby versions

Rebased and awaiting workflow approval, thank you.

@agirling
Copy link
Contributor Author

I'll dig a little deeper into the 2.4 failures. Keyword args came in to ruby 2.0, so I was not expecting an issue on the 2.4 path.

Thanks for getting GitHub actions online for this project.

@geekq
Copy link
Owner

geekq commented May 27, 2022

You could also add other intermediate Ruby versions (2.5, 2.6) to https://github.com/geekq/workflow/blob/develop/.github/workflows/test.yml#L15 so we can see, when it became incompatible. Then we can read release notes of the Ruby version, which changed things etc.

If you rebase your own develop branch in agirling/workflow, you should be able to run the GitHub Actions (tests) youself.

BTW, the agirling-ruby3-kwargs is not rebased, but merged the changes. Instead, while on your PR branch, please

git remote add upstream https://github.com/geekq/workflow.git
git remote -v
git fetch --all
git rebase upstream develop
git push --force <your-remote-e-g-origin>

If in your PR you check the checkbox "[ ] Allow edits from maintainers", I could do it as well or provide changes.

@geekq
Copy link
Owner

geekq commented May 27, 2022

If after reading Ruby release notes / investigating etc., it comes out, that an incompatible change is needed, please also describe the motivation of this PR: what is exactly the win for the users, which would justify introducing a new incompatible major version 3.0 for the workflow. Is it some Ruby3 syntactic sugar? Solving incompatibility with Ruby3? Could you please elaborate on that?

@agirling
Copy link
Contributor Author

The motivation for this change is Ruby 2.6 is EOL, and Ruby 2.7 has entered security security maintenance with an anticipated EOL of 2023-03-31. We initially encountered deprecation in 2.7 and ignored it, but have been forced to deal with the change as we prepare our applications for Ruby 3.

In my research and testing the separation of positional and keyword args begins a deprecation warning in 2.7 with support for the future behavior. I did a spike to see if I could use a RUBY_VERSION conditional to direct the appropriate behavior, but this ended up being a mess. The appropriate choice forward would be a new major release of the gem, as it is a breaking API change. It appears in the ruby discussions around this change, it was a bit of a rocky one to roll out.

We're comfortable running on a fork to support our Ruby 3 applications, if a new major release is not on the horizon for this project. Our motivation for the PR is to contribute our findings back to you and the community.

Thanks for your excellent work, and I appreciate your guidance through GitHub/PR process. Our workflow is entirely in GitLab, which is similar enough - but not what I do every day. I hope to increase our frequency of communications back to the community.

@geekq
Copy link
Owner

geekq commented May 27, 2022

Thanks for your elaborate explanation! Will quote/link it in release notes. Will prepare a major gem release soon...

Will also use GitHub action's fail_fast: false to run the tests for all Ruby versions and not stop on the first failure.

@geekq geekq merged commit bc5b6d4 into geekq:develop May 29, 2022
@geekq
Copy link
Owner

geekq commented May 29, 2022

https://rubygems.org/gems/workflow/versions/3.0.0 released, available for testing

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.

None yet

2 participants