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

Fix unmarshallable keys #52

Merged
merged 2 commits into from
Aug 22, 2024

Conversation

jan4843
Copy link
Contributor

@jan4843 jan4843 commented Aug 22, 2024

No description provided.

Marshalling/unmarshalling does not work when definitions cannot be marshalled (Lambda, for example).
Changelog.md Outdated
@@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

## [1.1.2] - 2024-08-22
### Fixed
- Fixed `Keys` containing non-marshallable definitions
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
- Fixed `Keys` containing non-marshallable definitions
- Fixed "TypeError: no _dump_data is defined for class Proc" error that ocurred when a definition model inherits from another model that uses lambda based definitions

@@ -5,7 +5,7 @@
describe Definition::Model do
let(:test_model_class) do
Class.new(described_class) do
required :name, Definition.Type(String)
required :name, Definition.NonEmptyString
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
required :name, Definition.NonEmptyString
required :name, Definition.Lambda { |value| conform_with(value.trim) }

to have a test case that would cause the error that this pr fixes

@Goltergaul Goltergaul added the bug label Aug 22, 2024
@Goltergaul Goltergaul self-assigned this Aug 22, 2024
@jan4843 jan4843 force-pushed the fix-unmarshallable-keys branch from 1cc8fa5 to 5badb46 Compare August 22, 2024 12:06
@Goltergaul Goltergaul merged commit bcd9e2c into Goltergaul:master Aug 22, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants