Skip to content

Conversation

@palkan
Copy link
Owner

@palkan palkan commented Jun 16, 2025

Context

This PR tracks the changes for v2.

What's inside

Checklist:

  • I have added tests
  • I have made corresponding changes to the documentation

spec.files = Dir.glob("lib/**/*") + %w[LICENSE.txt README.md]
spec.require_paths = ["lib"]
spec.required_ruby_version = ">= 2.7"
spec.required_ruby_version = ">= 3.1"

Choose a reason for hiding this comment

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

Data class is available since 3.2

Copy link
Owner Author

Choose a reason for hiding this comment

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

Good catch, thanks!

Copy link
Owner Author

Choose a reason for hiding this comment

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

Though the library can still be used with older Rubies (without data); we have a note in the Readme

class Engine < ::Rails::Engine
config.downstream = Downstream.config

::GlobalID::Locator.use "downstream" do |gid|

Choose a reason for hiding this comment

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

Feels like a better place for this code.

end
end

def define(*fields, &)

Choose a reason for hiding this comment

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

attributes and and define feel to me like different strategies to be applied or configured on the Event class.

I do like the new public method, but I think the public interface with this definition doesn't clearly state what's going on, that one variant is mutable and the other immutable. Perhaps the strategy could be defined through an attributes parameter, like immutable: true.

@palkan palkan marked this pull request as ready for review August 20, 2025 22:18
@palkan palkan merged commit 488154d into master Aug 20, 2025
6 checks passed
@palkan palkan deleted the v2-dev branch August 20, 2025 22:23
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.

3 participants