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

[Ldap] Add exception for mapping ldap errors #31547

Merged

Conversation

Simperfit
Copy link
Contributor

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #28677
License MIT
Doc PR

Maybe we could add more exception code since the list has a lot of errors, maybe we could add a class that maps the error to the right exeptions. see https://www.php.net/manual/en/function.ldap-errno.php

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

ping @lyrixx since you're mentioned as author

src/Symfony/Component/Ldap/Adapter/ExtLdap/Connection.php Outdated Show resolved Hide resolved
src/Symfony/Component/Ldap/Adapter/ExtLdap/Connection.php Outdated Show resolved Hide resolved
@Simperfit Simperfit force-pushed the feature/add-ldap-error-code-to-execption branch 2 times, most recently from d601edc to 792fdb4 Compare May 20, 2019 14:05
@lyrixx
Copy link
Member

lyrixx commented May 22, 2019

ping @lyrixx since you're mentioned as author

I did do any LDAP for a while. I can not help here. Sorry

@Simperfit Simperfit force-pushed the feature/add-ldap-error-code-to-execption branch from 792fdb4 to 1b29cb1 Compare May 22, 2019 16:30
nicolas-grekas added a commit that referenced this pull request May 22, 2019
This PR was merged into the 3.4 branch.

Discussion
----------

[Ldap] minor: fix phpdocs in the ldap component

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | none   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | <!-- required for new features -->

<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.

Additionally (see https://symfony.com/roadmap):
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the master branch.
-->

Fix just some phpdocs highlighted by @nicolas-grekas in #31547.

Commits
-------

721915f minor: fix phpdocs in the ldap component
@nicolas-grekas nicolas-grekas changed the base branch from master to 4.4 June 2, 2019 20:11
@Simperfit
Copy link
Contributor Author

@fabpot this is ready

@csarrazi
Copy link
Contributor

LGTM 👍

@fabpot
Copy link
Member

fabpot commented Jun 22, 2019

Thank you @Simperfit.

@fabpot fabpot merged commit 1b29cb1 into symfony:4.4 Jun 22, 2019
fabpot added a commit that referenced this pull request Jun 22, 2019
This PR was merged into the 4.4 branch.

Discussion
----------

[Ldap] Add exception for mapping ldap errors

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #28677   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | <!-- required for new features -->

<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.

Additionally (see https://symfony.com/roadmap):
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the master branch.
-->

Maybe we could add more exception code since the list has a lot of errors, maybe we could add a class that maps the error to the right exeptions. see https://www.php.net/manual/en/function.ldap-errno.php

Commits
-------

1b29cb1 [Ldap] Add exception for mapping ldap errors
@Simperfit Simperfit deleted the feature/add-ldap-error-code-to-execption branch June 22, 2019 08:35
fabpot added a commit that referenced this pull request Jul 3, 2019
…aviereguiluz)

This PR was merged into the 4.4 branch.

Discussion
----------

[Ldap] Document the new exceptions thrown by the code

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | -
| License       | MIT
| Doc PR        | not needed

While deciding if we should document #31547 I saw that the new exceptions weren't documented in the code. I don't know if this is the place where this should be added.

Commits
-------

0d27af9 [Ldap] Document the new exceptions thrown by the code
@nicolas-grekas nicolas-grekas removed this from the next milestone Oct 27, 2019
@nicolas-grekas nicolas-grekas added this to the 4.4 milestone Oct 27, 2019
This was referenced Nov 12, 2019
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.

6 participants