Skip to content

fix: source and context not passed to callable transformer #192

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.

Sign up for GitHub

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

Jump to bottom
Merged
Korbeil merged 1 commit into from
Oct 5, 2024
Merged

fix: source and context not passed to callable transformer #192

Korbeil merged 1 commit into from
Oct 5, 2024

Conversation

@MrMeshok
Copy link
Contributor

@MrMeshok MrMeshok commented Oct 3, 2024

Fixes #191

@MrMeshok MrMeshok marked this pull request as draft October 3, 2024 12:15
@MrMeshok MrMeshok marked this pull request as ready for review October 3, 2024 13:12
Korbeil
Korbeil approved these changes Oct 4, 2024
View reviewed changes
Copy link
Member

@Korbeil Korbeil left a comment

Choose a reason for hiding this comment

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

Looks good, could you add a test to validate the test ? 🙏

@MrMeshok
Copy link
Contributor Author

MrMeshok commented Oct 4, 2024

It's already should be tested, because class Bar is used in generic MapTo/MapFrom tests

@Korbeil
Copy link
Member

Korbeil commented Oct 5, 2024

I mean, it should fail if it is tested, you didn't change a test in this pull request and clearly it didn't failed before.

@MrMeshok
Copy link
Contributor Author

MrMeshok commented Oct 5, 2024

I changed tests/Fixtures/MapTo/Bar.php now transformC and transformDStatic requires source and context. Also I added transformer: 'htmlspecialchars' to $form property so usage of internal functions tested too. Without fix in CallableTransformer test will fail with this additions to Bar

Korbeil
Korbeil approved these changes Oct 5, 2024
View reviewed changes
Copy link
Member

@Korbeil Korbeil left a comment

Choose a reason for hiding this comment

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

Thanks for the test and your contribution! @MrMeshok 🙏

@Korbeil Korbeil merged commit 9d7523d into jolicode:main Oct 5, 2024
@MrMeshok MrMeshok deleted the fix-191 branch October 5, 2024 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Korbeil Korbeil Korbeil approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Source and context parameters not passed to transformer ?

2 participants

@MrMeshok @Korbeil