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

Further exceptions tweaks #7210

Closed
wants to merge 5 commits into from
Closed

Conversation

Majkl578
Copy link
Contributor

@Majkl578 Majkl578 commented May 2, 2018

Expands and refines #6743.

  • All exceptions are made final (or abstract).
  • Nameless constructors renamed to new().
  • Exceptions moved around to their appropriate places (Exception sub-namespace), except few that are to be handled later (i.e. Mapping, Query).
  • Removed Exception suffixes.
  • Also cleaned up root namespace by moving out non-namespaced exceptions.

To be documented after first round of review.

cc @greg0ire

TODO:

  • add method constructors for pre-3.0 exceptions

@Majkl578 Majkl578 added this to the 3.0 milestone May 2, 2018
/**
* @return PessimisticLockFailed
*/
public static function lockFailed()
Copy link
Contributor

Choose a reason for hiding this comment

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

Return type maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably not in this PR, not directly related. :)

@Majkl578 Majkl578 force-pushed the exceptions-tweaks branch from 97477bd to d7b4d39 Compare May 2, 2018 01:57
@Majkl578 Majkl578 force-pushed the exceptions-tweaks branch from d7b4d39 to bb286d4 Compare May 2, 2018 02:09
namespace Doctrine\ORM\Exception;

/**
* Exception for a unexpected query result.
Copy link
Member

Choose a reason for hiding this comment

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

a => an

@@ -14,8 +14,10 @@

/**
* A MappingException indicates that something is wrong with the mapping setup.
*
* @todo Split into specific exceptions and change to interface.
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

Beautiful! ✨

@@ -8,6 +8,8 @@

/**
* Exception for a unexpected query result.
*
* @final
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for having this as an annotation?

Copy link
Member

Choose a reason for hiding this comment

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

Nevermind 😛

result contains more than one object, an ``NonUniqueResultException``
is thrown. If the result contains no objects, an ``NoResultException``
is thrown. The pure/mixed distinction does not apply.
result contains more than one object, an ``NonUniqueResult``

Choose a reason for hiding this comment

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

an => a

is thrown. If the result contains no objects, an ``NoResultException``
is thrown. The pure/mixed distinction does not apply.
result contains more than one object, an ``NonUniqueResult``
is thrown. If the result contains no objects, an ``NoResult``

Choose a reason for hiding this comment

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

an => a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants