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

Typecast refactoring #295

Merged
merged 16 commits into from
Jul 16, 2023
Merged

Typecast refactoring #295

merged 16 commits into from
Jul 16, 2023

Conversation

Tigrov
Copy link
Member

@Tigrov Tigrov commented Jul 4, 2023

Q A
Is bugfix?
New feature?
Breaks BC?
Fixed issues

@what-the-diff
Copy link

what-the-diff bot commented Jul 4, 2023

PR Summary

  • Ignoring PHPUnit Cache Files
    We've updated the system to ignore caching files from PHPUnit, a testing tool, to keep our codebase clean and efficient.
  • Refactoring Code in ColumnSchema.php
    • We have rewritten some sections of code using newer features in PHP, specifically match expressions, for more clear code in two methods: phpTypecast and dbTypecast.
  • Updating Column Default Value Handling in Schema.php
    • We've removed redundant tasks and added a new method to effectively handle and convert the default values of database columns, making the application more efficient.
  • Test Updates Reflecting Code Changes
    • Made necessary changes in our testing codes, SchemaProvider.php and SchemaTest.php, so that they reflect our software updates.
  • Fixture File Update
    • We've updated a fixture file named mysql.sql, which is used for testing, specifically for the type_bit table to ensure our tests can properly validate our latest changes.

@codecov
Copy link

codecov bot commented Jul 6, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01 ⚠️

Comparison is base (9db293e) 99.12% compared to head (afa157b) 99.11%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #295      +/-   ##
============================================
- Coverage     99.12%   99.11%   -0.01%     
+ Complexity      192      191       -1     
============================================
  Files            13       13              
  Lines           570      565       -5     
============================================
- Hits            565      560       -5     
  Misses            5        5              
Impacted Files Coverage Δ
src/ColumnSchema.php 100.00% <100.00%> (ø)
src/Schema.php 99.63% <100.00%> (-0.01%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Tigrov Tigrov marked this pull request as ready for review July 6, 2023 07:25
@terabytesoftw terabytesoftw added the status:code review The pull request needs review. label Jul 6, 2023
Copy link
Contributor

@darkdef darkdef left a comment

Choose a reason for hiding this comment

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

  1. Two type of changes in code: swap methods order and logical refactoring. Need split
  2. match(true) - I think this bad practice and not approved this changes. Early exit work fastest.

@Tigrov
Copy link
Member Author

Tigrov commented Jul 9, 2023

  1. Two type of changes in code: swap methods order and logical refactoring. Need split
  2. match(true) - I think this bad practice and not approved this changes. Early exit work fastest.
  1. Ok, agree. Thanks.
  2. Why do you think this bad practice?

Early exit work fastest.

In the case => $value equals return $value

For me match (true) looks better than a lot of if
https://github.com/yiisoft/yii-queue/blob/master/src/Debug/QueueCollector.php#L37
https://github.com/yiisoft/log/blob/master/src/Message/Formatter.php#L148
https://github.com/yiisoft/yii-widgets/blob/master/src/Dropdown.php#L639

@darkdef
Copy link
Contributor

darkdef commented Jul 9, 2023

2. Why do you think this bad practice?

Match with short and simple condition - good solution (as examples from other packages), but in this case we have complex conditions.
It's my opinion.

@darkdef darkdef requested a review from samdark July 9, 2023 10:27
src/Schema.php Outdated Show resolved Hide resolved
@Tigrov Tigrov requested a review from darkdef July 16, 2023 08:55
@darkdef darkdef merged commit 04a6a66 into yiisoft:master Jul 16, 2023
39 of 40 checks passed
@darkdef
Copy link
Contributor

darkdef commented Jul 16, 2023

Great work. Thanks

@Tigrov Tigrov deleted the typecast_refactoring branch July 17, 2023 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:code review The pull request needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants