state-machines / state_machines-audit_trail

Log transitions on a state_machines gem to support auditing and business process analytics.
https://github.com/state-machines/state_machines-audit_trail
MIT License
75 stars 24 forks source link

feat: Eliminate usage of OpenStruct #42

Closed casperisfine closed 10 months ago

casperisfine commented 10 months ago

OpenStruct design cause it to have non-local performance impact, especially with JIT compilers.

See https://github.com/ruby/ostruct/issues/51 for full context, and it Ruby 3.3 OpenStruct usage raise a performance warnings:

gems/state_machines-audit_trail-2.0.2/lib/state_machines/audit_trail/transition_auditing.rb:38:
warning: OpenStruct use is discouraged for performance reasons

In this case I don't really see any advantage of OpenStruct over a simple PORO.

cc @seuros

seuros commented 10 months ago

I will add testing in ruby 3.3 in another PR

casperisfine commented 10 months ago

Alright, after adding a few more empty methods to comply to the Transition interface, it's now passing.

I believe that strictly speaking this is backward compatible, since the initial transition accepting any possible method was AFAIK not documented. However it's not impossible some users may need to fix their code if they were wrongly calling methods outside of the interface.

If we want to preserve backward compatibility for these projects, we can also define a method_missing that just returns nil.

seuros commented 10 months ago

I will add the release workflow and merge this PR.

casperisfine commented 10 months ago

Thank you very much 🙇

seuros commented 10 months ago

released