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

Fixes #500 . If precision is 0 command adds it anyway. #562

Merged
merged 10 commits into from
Sep 25, 2022

Conversation

ajibarra
Copy link
Member

@ajibarra ajibarra commented Sep 16, 2022

Changing empty to is_null avoids ignoring decimal(X,0) and snapshot is generated correctly. Fixes #500

@ajibarra
Copy link
Member Author

ajibarra commented Sep 16, 2022

@ADmad Same situation than #561

psalm reports:

Error: vendor/laminas/laminas-diactoros/src/functions/marshal_headers_from_sapi.php:29:18: ParseError: Syntax error, unexpected T_STRING, expecting T_PAAMAYIM_NEKUDOTAYIM on line 29 (see https://psalm.dev/173)
Error: vendor/laminas/laminas-diactoros/src/functions/marshal_headers_from_sapi.php:29:28: ParseError: Syntax error, unexpected T_VARIABLE, expecting ')' on line 29 (see https://psalm.dev/173)

If you have any clue about how to fix it, it is more than welcome :)

See #561 (comment)

@ajibarra ajibarra marked this pull request as ready for review September 16, 2022 06:55
@markstory
Copy link
Member

If you have any clue about how to fix it, it is more than welcome :)

When I've run into this in the past it was the php version being too low in composer.json. Try bumping it to >=7.4.0.

@ajibarra
Copy link
Member Author

If you have any clue about how to fix it, it is more than welcome :)

When I've run into this in the past it was the php version being too low in composer.json. Try bumping it to >=7.4.0.

Hey @markstory, yes it may fix the issue as well, I've fixed changing coding standards php to 7.2 per Admad's recommendation and it worked. However if you prefer increasing php requirements to 7.4 I can make that change.

Thanks you!

@markstory
Copy link
Member

However if you prefer increasing php requirements to 7.4 I can make that change.

I think that's a safe change to make as 4.4 also requires php7.4.

@ajibarra ajibarra force-pushed the issue/500-decimal-no-precision branch from 595d098 to 8a1ea70 Compare September 23, 2022 07:04
@dereuromark
Copy link
Member

dereuromark commented Sep 23, 2022

Looks good!

@markstory markstory merged commit 8ba15cc into cakephp:3.x Sep 25, 2022
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.

Migration snapshot not working on decimal with zero scale
3 participants