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

test cases typo fixed & missing PHPDocs added. #324

Closed
wants to merge 1 commit into from

Conversation

MrWasimAbbasi
Copy link

@MrWasimAbbasi MrWasimAbbasi commented Mar 31, 2024

Q A
Is bugfix?
New feature?
Breaks BC?
Fixed issues Typo fixes and missing PHPDoc added

Copy link

what-the-diff bot commented Mar 31, 2024

PR Summary

  • Enhancements to the Column class
    A new method asString() was added to allow for better representation of the object.

  • Expansion of capabilities for the Command class
    insertWithReturningPks() and showDatabases() methods were added. These provide more functionality for handling database commands.

  • Richer control over database connections with the Connection class improvements
    Several new methods were introduced for creating commands, handling queries and managing database schemas.

  • Facilitating driver usage in Driver class
    A createConnection method was introduced to make creating new connections simpler.

  • Better value management in the Quoter class
    A method for quoting values, quotevalue(), is now available.

  • Testing improvements to verify new functionalities.
    Various classed like ColumnSchemaTest, PdoConnectionTest, QueryBuilderJsonTest, QueryBuilderTest, QueryTest, QuoterTest, and SchemaTest have new testing methods. These methods are designed to confirm that new changes around type casting, JSON queries, query building, and database schemas work as expected.

  • Expanding tests for different scenarios in database handling
    Tests to verify the workings of different functionalities like handling tinyInt(1) column type, inserting default values, handling non-connection PDO, and more have been added to the SchemaTest class.

Copy link

codecov bot commented Mar 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.04%. Comparing base (4f44e1a) to head (d7ed03f).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #324   +/-   ##
=========================================
  Coverage     99.04%   99.04%           
  Complexity      174      174           
=========================================
  Files            13       13           
  Lines           524      524           
=========================================
  Hits            519      519           
  Misses            5        5           

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

@Tigrov
Copy link
Member

Tigrov commented Mar 31, 2024

Thanks for the PR, but the changes do not seem to make sense.

See https://github.com/yiisoft/docs/blob/master/010-code-style.md#comments

Method comment is necessary except it adds nothing to what method name and signature already has.

@vjik
Copy link
Member

vjik commented Mar 31, 2024

It's make sense for exceptions, that need to catch in code, that usage it method.

@Tigrov
Copy link
Member

Tigrov commented Mar 31, 2024

It's make sense for exceptions, that need to catch in code, that usage it method.

Then when possible and reasonably it is better to add them to interfaces.

@@ -51,6 +51,9 @@ protected function buildCommentString(): string
return ' COMMENT ' . (string) (new Quoter('`', '`'))->quoteValue($this->getComment());
}

/**
* @return string
Copy link
Member

Choose a reason for hiding this comment

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

Not needed since it's obvious from return type and there's no custom description.

@samdark
Copy link
Member

samdark commented Apr 2, 2024

Closing since we're not going to apply the pull request. Thanks for submitting it anyway.

@samdark samdark closed this Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants