Skip to content

[WIP] [Ldap] Marked the Ldap component as internal #16735

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

Merged
merged 1 commit into from
Nov 29, 2015
Merged

[WIP] [Ldap] Marked the Ldap component as internal #16735

merged 1 commit into from
Nov 29, 2015

Conversation

csarrazi
Copy link
Contributor

Q A
Bug fix? no
New feature? no
BC breaks? yes
Deprecations? no
Tests pass? no
Fixed tickets no
License MIT
Doc PR not yet

As mentioned earlier, the LDAP component suffers from a few problems, addressed in PR #15994
However, as raised by @Tobion, the component does not yet have tests (they can be added at a later time, though), and is not considered stable yet.

*/
interface LdapClientInterface
{
const LDAP_ESCAPE_FILTER = 0x01;
const LDAP_ESCAPE_DN = 0x02;
Copy link
Member

Choose a reason for hiding this comment

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

not sure about this change: using the ext-ldpa constants binds the interface to the extension, thus removes any genericness don't you think?

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, but these were only here for the ldap_escape polyfill.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nicolas-grekas Moreover, we want to prevent people from being highly coupled to an interface which will introduce BC at a later time. We may reintroduce them at a later date.

@fabpot
Copy link
Member

fabpot commented Nov 29, 2015

Thank you @csarrazi.

@fabpot fabpot merged commit 3f89b2c into symfony:2.8 Nov 29, 2015
fabpot added a commit that referenced this pull request Nov 29, 2015
…arrazi)

This PR was merged into the 2.8 branch.

Discussion
----------

[WIP] [Ldap] Marked the Ldap component as internal

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | yes
| Deprecations? | no
| Tests pass?   | no
| Fixed tickets | no
| License       | MIT
| Doc PR        | not yet

As mentioned earlier, the LDAP component suffers from a few problems, addressed in PR #15994
However, as raised by @Tobion, the component does not yet have tests (they can be added at a later time, though), and is not considered stable yet.

Commits
-------

3f89b2c Marked the Ldap component as internal and removed Ldap constants polyfill
This was referenced Nov 30, 2015
@csarrazi csarrazi deleted the feature/ldap-component-as-internal branch January 27, 2016 11:04
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.

4 participants