Skip to content

Conversation

rustamwin
Copy link
Member

@rustamwin rustamwin commented Aug 18, 2025

Copy link

codecov bot commented Aug 18, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.41%. Comparing base (8f45076) to head (c94a597).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #1038   +/-   ##
=========================================
  Coverage     98.41%   98.41%           
- Complexity     1644     1647    +3     
=========================================
  Files           117      117           
  Lines          4344     4353    +9     
=========================================
+ Hits           4275     4284    +9     
  Misses           69       69           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rustamwin rustamwin requested review from vjik, Tigrov and a team August 19, 2025 04:28
@rustamwin rustamwin marked this pull request as ready for review August 20, 2025 12:38
@rustamwin rustamwin requested review from Tigrov and a team August 20, 2025 12:45
@Tigrov
Copy link
Member

Tigrov commented Aug 20, 2025

And needs to fix active-record and db-migration tests

@Tigrov
Copy link
Member

Tigrov commented Aug 20, 2025

Also needs to add tests with execution. Not all DBMS support FROM clause in UPDATE query (for example MySQL)

Copy tests from QueryBuilderProvider::update() to CommandProvider::update()

return [$uniqueNames, $insertNames, null];
}

protected function prepareFromTables(array|ExpressionInterface|string $from): array
Copy link
Member

Choose a reason for hiding this comment

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

This method identical to Query::from(). Can we remove duplicates?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we can but we need to refactor a bit.

@vjik vjik requested a review from a team September 6, 2025 05:05
@vjik vjik added the status:code review The pull request needs review. label Sep 6, 2025
@Tigrov Tigrov merged commit 2380447 into master Sep 22, 2025
70 checks passed
@Tigrov Tigrov deleted the fix-94 branch September 22, 2025 09:19
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