Skip to content

Conversation

@chhtz
Copy link
Member

@chhtz chhtz commented Dec 11, 2025

This allows calling it via send in @explicit_activity without deprecation warnings in ruby 2.7, and failures in ruby 3.2

@pierrewillenbrockdfki FYI

This allows calling it via `send` in `@explicit_activity`
without deprecation warnings in ruby 2.7, and failures in ruby 3.2
@chhtz chhtz requested review from doudou and jhonasiv December 11, 2025 16:34
@pierrewillenbrockdfki
Copy link
Contributor

Alternative implementations could teach default_activity about keyword arguments or pass a block. But in my opinion, only allowing positional arguments is fine.

@doudou
Copy link
Member

doudou commented Dec 15, 2025

Alternative implementations could teach default_activity about keyword arguments or pass a block. But in my opinion, only allowing positional arguments is fine.

Not in mine. We're using that parameter, we'd have to change everywhere it is actually used.

@doudou
Copy link
Member

doudou commented Dec 15, 2025

The right thing is to fix the forwarding of default_activity.

@chhtz
Copy link
Member Author

chhtz commented Dec 15, 2025

The right thing is to fix the forwarding of default_activity.

I'm fine with that as well. My ruby skills are a bit limited for a proper fix then, however.
Feel free to overwrite this branch or make a new PR @doudou

@pierrewillenbrockdfki
Copy link
Contributor

pierrewillenbrockdfki commented Jan 5, 2026

Happy new year everyone!

@doudou I found

if kw.empty?
@spec.send(m, *args, &block)
else
@spec.send(m, *args, **kw, &block)

Is there a specific reason to not just always splat **kw? It seems to work fine in ruby 2.3.

@pierrewillenbrockdfki
Copy link
Contributor

Currently testing pierrewillenbrockdfki@72a843c -- i believe this is already being used in our CI for Ubuntu 22.04 @chhtz does that work for your Ubuntu 24.04?

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.

4 participants