Doctrine DBAL
  1. Doctrine DBAL
  2. DBAL-867

Doctrine\DBAL\Driver\Connection should be marked as an internal interface

    Details

    • Type: Documentation Documentation
    • Status: Reopened
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Security Level: All
    • Labels:
      None

      Description

      Currently, the main entry point Doctrine\DBAL\Connection is documented as a wrapper around Doctrine\DBAL\Driver\Connection.
      This is very misleading and encourages other library to typehint against Doctrine\DBAL\Driver\Connection rather than Doctrine\DBAL\Connection. See https://github.com/symfony/symfony/pull/10720 for the original discussion.

      However, the discussion in https://github.com/doctrine/dbal/pull/414#discussion_r7688765 implies that they should actually not be related together (but it cannot be fixed for BC reasons). The phpdoc should at least be changed

        Activity

        Hide
        Guilherme Blanco added a comment -

        This issue was fixed some time ago.

        Commit reference: https://github.com/doctrine/dbal/commit/5fdedc4f8f304e8035580807bd36d59e97a87265

        Show
        Guilherme Blanco added a comment - This issue was fixed some time ago. Commit reference: https://github.com/doctrine/dbal/commit/5fdedc4f8f304e8035580807bd36d59e97a87265
        Hide
        Steve Müller added a comment -

        Guilherme Blanco How is your commit reference related to this issue?

        Show
        Steve Müller added a comment - Guilherme Blanco How is your commit reference related to this issue?
        Hide
        Guilherme Blanco added a comment - - edited

        Steve Müller The suggestion about Doctrine\DBAL\Connection and Doctrine\DBAL\Driver\Connection started around the misleading support of ping and how to consistently support it across different drivers.
        The commit reference I pointed out is Benjamin Eberlei's resolution to remove misleading phpdoc around ping support (which is also highlighted in the ticket as "phpdoc should at least be changed").
        If there's anything else that is missing, I'm probably not seeing. All I've done is followed dbal's discussion. =\

        Show
        Guilherme Blanco added a comment - - edited Steve Müller The suggestion about Doctrine\DBAL\Connection and Doctrine\DBAL\Driver\Connection started around the misleading support of ping and how to consistently support it across different drivers. The commit reference I pointed out is Benjamin Eberlei 's resolution to remove misleading phpdoc around ping support (which is also highlighted in the ticket as "phpdoc should at least be changed"). If there's anything else that is missing, I'm probably not seeing. All I've done is followed dbal's discussion. =\
        Hide
        Christophe Coevoet added a comment -

        Your commit does not fix it at all. Benjamin Eberlei's comment was about the ping method only indeed. But he explained that Doctrine\DBAL\Connection should actually not be a Doctrine\DBAL\Driver\Connection except for legacy reasons, which is why makign it implement Doctrine\DBAL\Driver\PingableConnection was a bad idea even if it has a ping method.

        My issue is related to the description of the class itself: https://github.com/doctrine/dbal/blob/aa2ed45ade6582a24e4f72f674f6989873d72112/lib/Doctrine/DBAL/Connection.php#L36
        It still describes it as a wrapper around the driver connection, making other devs think that the right typehint in other libraries is the internal driver connection. See the discussion in the Symfony PR I linked

        Show
        Christophe Coevoet added a comment - Your commit does not fix it at all. Benjamin Eberlei 's comment was about the ping method only indeed. But he explained that Doctrine\DBAL\Connection should actually not be a Doctrine\DBAL\Driver\Connection except for legacy reasons, which is why makign it implement Doctrine\DBAL\Driver\PingableConnection was a bad idea even if it has a ping method. My issue is related to the description of the class itself: https://github.com/doctrine/dbal/blob/aa2ed45ade6582a24e4f72f674f6989873d72112/lib/Doctrine/DBAL/Connection.php#L36 It still describes it as a wrapper around the driver connection, making other devs think that the right typehint in other libraries is the internal driver connection. See the discussion in the Symfony PR I linked
        Hide
        Guilherme Blanco added a comment -

        So... it's reopened. I'll look into this later today.

        Show
        Guilherme Blanco added a comment - So... it's reopened. I'll look into this later today.
        Hide
        Marco Pivetta added a comment -

        Guilherme Blanco can you re-check this?

        Show
        Marco Pivetta added a comment - Guilherme Blanco can you re-check this?

          People

          • Assignee:
            Guilherme Blanco
            Reporter:
            Christophe Coevoet
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated: