Skip to content

Make proc_to_ast dependency optional #15

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

Conversation

viralpraxis
Copy link

Resolves #14

@sue445 sue445 self-requested a review February 16, 2025 22:48
Copy link
Contributor

@sue445 sue445 left a comment

Choose a reason for hiding this comment

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

#14 says

which does not work well with Ruby 3.4 syntax.

But, all tests on the main branch pass.
https://github.com/rspec-parameterized/rspec-parameterized-core/actions/runs/12527144702/job/34940428274

Please add to your test code what specific syntax causes the error

@viralpraxis
Copy link
Author

parser is gradually becoming obsolete due to the new built-in prism parser. As far as I know, it will be deprecated in the long run. And compatibility with newer ruby versions is not guaranteed. Perhaps I should try to resolve the issue on the proc_to_ast's side (by making it aware of prism). If that works out, there won't be any required changes here.

@sue445
Copy link
Contributor

sue445 commented Feb 17, 2025

@viralpraxis

parser is gradually becoming obsolete due to the new built-in prism parser

I agree too.

However, the information you wrote does not include the minimum code that reproduces the error (In this case, it will be the test case mentioned in the #15 (review)) or the specific error.

Looking at just this diff, it looks like you are modifying code that works fine.

Therefore I cannot accept your patch.

@viralpraxis
Copy link
Author

Yep, your point make sense. I'll close the issue since that's another's gem problem and it should be fixed there. If that does not happen, we can revisit it later.

@viralpraxis viralpraxis deleted the make-proc-to-ast-dependency-optional branch February 17, 2025 01:17
@viralpraxis viralpraxis restored the make-proc-to-ast-dependency-optional branch February 17, 2025 19:19
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.

Make proc-to-ast dependency optional
2 participants