Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Optimize abstract platform #291

Closed
wants to merge 8 commits into from

6 participants

Steve Müller doctrinebot Christophe Coevoet Marco Pivetta Guilherme Blanco Benjamin Eberlei
Steve Müller
Collaborator

This PR optimizes the AbstractPlatform class in the following ways:

  • Add dedicated use statemtent for each class import
  • Add missing class imports
  • Add missing phpDoc blocks for methods
  • Fix existing phpDocs (parameters, return types, @throws tags, FQDN types, indentation etc.)
  • "Normalize" phpDocs (consistent verbs for what a method does, reuse of certain phrases for similar method types etc.)
  • Fix CS
  • Improve certain code portions

I hope this is a welcomed improvement. It reads much better now and feels somewhat cleaner to me.

doctrinebot
Collaborator

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DBAL-470

Christophe Coevoet

You removed some information form the phpdoc in getListTableIndexesSQL and getDefaultTransactionIsolationLevel (the fact that it returns one of the constants).

And you should also revert the change from elseif to else if. We are writing it without space

Steve Müller
Collaborator

Readded the important information in phpDocs and fixed else if CS.

Steve Müller
Collaborator

BTW are the Doctrine coding standards written down anywhere? Or is there even a PHPCS template available for download?

Marco Pivetta
Owner
Guilherme Blanco

Easy.... =)

We follow Symfony2 Coding standards, but we made a strict decision to choose ! as an operator (which requires spaces around it).
Also, we follow and advocate Object Calisthenics as much as we can as long as it doesn't degrade performance.

Guilherme Blanco

The part that we don't have clear agreement is about Abstract classes and Interfaces.
Some like to prefix with Abstract or suffix with Interfaces... others consider it shouldn't have class modifiers into their names.
Until we clear this, we stick to Symfony2 coding standards. =)

I'm pro do not suffix or prefix, fyi...

Steve Müller
Collaborator

Hey thanks for clearing this up =). I guessed that Doctrine tries to go along with Symfony2 coding standards but I wasn't sure about the specific differences.

Marco Pivetta
Owner
Benjamin Eberlei
Owner

@deeky666 Can you rebase the PR? With 2.4 out we now have capacity for all your pending PRs :-)

Steve Müller
Collaborator

@beberlei great to hear that 2.4 is out! I have rebased this PR.

Benjamin Eberlei
Owner

Closing as rebase is too much effort at this point.

Benjamin Eberlei beberlei closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.