Skip to content

refactor: Reorganize transformInteriorValue #9566

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 2 commits into
base: alpha
Choose a base branch
from

Conversation

kassiansun
Copy link

@kassiansun kassiansun commented Jan 22, 2025

Pull Request

Issue

This code was checking a constantly true condition value !== CannotTransform, that's why the code was not properly handling nested structure properly, The code introduced by #8446 fixed this logic, by introducing another duplicated piece of code.

Closes: #9567

Approach

This commit simplifies the logic here to reflect the original purpose.

Copy link

I will reformat the title to use the proper commit message syntax.

@parse-github-assistant parse-github-assistant bot changed the title refactor: reorganize transformInteriorValue refactor: Reorganize transformInteriorValue Jan 22, 2025
Copy link

parse-github-assistant bot commented Jan 22, 2025

🚀 Thanks for opening this pull request!

@kassiansun kassiansun force-pushed the alpha branch 2 times, most recently from 9ec6cfd to d4c3acf Compare January 22, 2025 06:42
This code was checking a constantly true condition value !== CannotTransform,
that's why the code was not properly handling nested structure properly,
The code introduced by parse-community#8446 fixed this logic, by introducing another
duplicated piece of code.

This commit simplifies the logic here to reflect the original purpose.
@kassiansun
Copy link
Author

This PR should also fix the (broken since #8446) regexp and bytes conversion, althought I didn't try to test it

@mtrezza
Copy link
Member

mtrezza commented Apr 7, 2025

@kassiansun Thank you for the PR, and you patience in waiting for review. I've rebased it and restarted the CI.

Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

You removed a few handling cases, what is the purpose of this, shouldn't the removed cases be handled as well?

@kassiansun
Copy link
Author

You removed a few handling cases, what is the purpose of this, shouldn't the removed cases be handled as well?

These cases are handled later in the function, the logic was duplicated twice

@mtrezza
Copy link
Member

mtrezza commented Apr 11, 2025

Could you provide more details? Where are they handled?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

transformInteriorValue doesn't handle conversion properly
2 participants