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

Fix docblock typos in DriverManager docs #3834

Merged
merged 1 commit into from
Jan 22, 2020

Conversation

CHItA
Copy link
Contributor

@CHItA CHItA commented Jan 19, 2020

Q A
Type documentation
BC Break no
Fixed issues none

Summary

In the api docs the names of the drivers are incorrect. Also, could someone confirm that the (unstable) annotations are still correct?

@morozov
Copy link
Member

morozov commented Jan 19, 2020

@CHItA thanks for the update. The ibm_db2 driver is pretty much stable. The (unstable) annotation should definitely be removed from there. As for pdo_oci, it is usable and we continuously test against it. In the end, it's the consumer's call which driver to use.

I'd remove the (unstable) annotation from everywhere and sort the drivers alphabetically.

@@ -90,17 +90,17 @@ private function __construct()
*
* Either 'driver' with one of the following values:
*
* drizzle_pdo_mysql
Copy link
Member

Choose a reason for hiding this comment

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

Hm… it actually duplicates the keys of the driver map. Would it make sense to reference if using {@link $_driverMap} instead of duplication?

private static $_driverMap = [
'pdo_mysql' => PDOMySQLDriver::class,
'pdo_sqlite' => PDOSQLiteDriver::class,
'pdo_pgsql' => PDOPgSQLDriver::class,
'pdo_oci' => PDOOCIDriver::class,
'oci8' => OCI8Driver::class,
'ibm_db2' => DB2Driver::class,
'pdo_sqlsrv' => PDOSQLSrvDriver::class,
'mysqli' => MySQLiDriver::class,
'drizzle_pdo_mysql' => DrizzlePDOMySQLDriver::class,
'sqlanywhere' => SQLAnywhereDriver::class,
'sqlsrv' => SQLSrvDriver::class,
];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, I am fine with whatever. Although $_driverMap doesn't seem to be listed in the API docs, which I would consider to be quite useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One more note, then I'll be happy to push whatever you find to be the best solution:
a) Which one of these would you prefer? Using a {@link $_driverMap} or just linking to the getAvailableDrivers() function which returns the keys of this array.
b) The docblock has no return value annotation so if I'm here, I might as well fix that too. Should I?

Also, what is your commit squashing policy? Three commits feel a bit much for a one line documentation change.

Copy link
Member

Choose a reason for hiding this comment

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

Which one of these would you prefer?

{@link $_driverMap} seems like a more reasonable reference. It contains the actual values we're referring to.

The docblock has no return value annotation […]

Which one?

Also, what is your commit squashing policy? Three commits feel a bit much for a one line documentation change.

We usually squash commits unless they provide additional clarity to the changes (not the case here).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, and getConnection() is missing the return statement.

@CHItA CHItA force-pushed the fix-driver-manager-documentation branch from 3d6d354 to c12db80 Compare January 22, 2020 18:55
@morozov morozov added this to the 2.10.2 milestone Jan 22, 2020
@morozov morozov self-assigned this Jan 22, 2020
@morozov morozov merged commit 4343e06 into doctrine:2.10 Jan 22, 2020
@morozov
Copy link
Member

morozov commented Jan 22, 2020

Thanks, @CHItA!

@CHItA
Copy link
Contributor Author

CHItA commented Jan 22, 2020

@morozov No worries, thank you for maintaining Doctrine! :)

@CHItA CHItA deleted the fix-driver-manager-documentation branch January 22, 2020 19:10
rgrellmann added a commit to Rossmann-IT/dbal that referenced this pull request Apr 22, 2020
Release [2.10.2](https://github.com/doctrine/dbal/milestone/75)

2.10.2
======

- Total issues resolved: **4**
- Total pull requests resolved: **19**
- Total contributors: **10**

Improvement,Static Analysis
---------------------------

 - [3964: Mark every exception as immutable](doctrine#3964) thanks to @greg0ire

CI,Improvement,Static Analysis
------------------------------

 - [3961: Stop relying on the master version of Psalm](doctrine#3961) thanks to @greg0ire
 - [3951: Setup static analysis with Psalm](doctrine#3951) thanks to @greg0ire
 - [3799: Upgrade to PHPStan v0.12](doctrine#3799) thanks to @lcobucci

Improvement,Logging,Test Suite
------------------------------

 - [3957: Reworked LoggingTest to be able to test Statement::executeUpdate()](doctrine#3957) thanks to @morozov

CI,Code Style,Improvement,Strict Typing
---------------------------------------

 - [3955: Remove baseline](doctrine#3955) thanks to @greg0ire

Bug,SQLite,Schema Introspection,Schema Managers
-----------------------------------------------

 - [3937: Column comment incorrectly introspected on SQLite](doctrine#3937) thanks to @morozov

Bug,Documentation,Prepared Statements,Query
-------------------------------------------

 - [3896: Updated documentation for QueryBuilder::execute() return value type](doctrine#3896) thanks to @morozov

Bug,Prepared Statements
-----------------------

 - [3894: Make sure that the $types array has the same keys $params](doctrine#3894) thanks to @morozov
 - [3893: Ensure the constructor arguments are passed to custom classes](doctrine#3893) thanks to @duncan3dc
 - [3843: Fix unquoted stmt fragments backslash escaping](doctrine#3843) thanks to @morozov

Documentation,Improvement
-------------------------

 - [3886: Update readme](doctrine#3886) thanks to @greg0ire
 - [3834: Fix docblock typos in DriverManager docs](doctrine#3834) thanks to @CHItA

CI,Improvement,MariaDB,MySQL
----------------------------

 - [3884: Use Docker consistently](doctrine#3884) thanks to @greg0ire
 - [3478: Improve readiness probe stability for containerized databases on CI](doctrine#3478) thanks to @morozov

 - [3883: Fix broken build](doctrine#3883) thanks to @greg0ire

Bug,Documentation,Query,Query Limit/Offset Modification
-------------------------------------------------------

 - [3842: Fixed the QueryBuilder::setMaxResults() signature to accept NULL](doctrine#3842) thanks to @morozov

Bug,Query
---------

 - [3832: Fix JOIN with no condition bug](doctrine#3832) thanks to @BenMorel

Bug,PostgreSQL,Schema Introspection
-----------------------------------

 - [3821: &doctrine#91;pg&doctrine#93; fix getting table information if search&doctrine#95;path contains escaped schema name](doctrine#3821) thanks to @linniksa

Documentation,Improvement,Logging
---------------------------------

 - [3812: Fix DebugStack#queries docblock type](doctrine#3812) thanks to @ostrolucky

Bug,Regression,Schema
---------------------

 - [3790: fixed unqualified table name of fk constraints when using schemas](doctrine#3790) thanks to @stlrnz and @Alarich

# gpg: Signature made Mon Apr 20 19:59:36 2020
# gpg:                using DSA key 2C3A645671828132
# gpg: Can't check signature: public key not found

# Conflicts:
#	README.md
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants