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

POD documentation inconsistencies #13

Closed
phillipod opened this issue May 4, 2015 · 2 comments
Closed

POD documentation inconsistencies #13

phillipod opened this issue May 4, 2015 · 2 comments

Comments

@phillipod
Copy link
Collaborator

Hi,

The following LDAPapi.pm functions:

  • add
  • add_s
  • modify
  • modify_s

have the following documentation signatures:

=item add DN ATTR SCTRLS CCTRLS
=item add_s DN ATTR SCTRLS CCTRLS
=item modify DN MOD
=item modify_s DN MOD

They have the following rearrange() patterns:

sub add { $self->rearrange(['DN', 'MOD', 'SCTRLS', 'CCTRLS'], @args); }
sub add_s { $self->rearrange(['DN', 'MOD', 'SCTRLS', 'CCTRLS'], @args); }
sub modify {$self->rearrange(['DN', 'MOD', 'SCTRLS', 'CCTRLS'], @args); }
sub modify_s { $self->rearrange(['DN', 'MOD', 'SCTRLS', 'CCTRLS'], @args); }

This impacts the usability of the documentation for the add() and add_s() functions, as the uppercased names seem to indicate the name for named parameter passing.

Changing the POD wouldn't break existing working code, but the API then becomes murky.
Changing the code (to e.g., 'ATTR' for add() and add_s()) breaks code only currently working through knowledge of internals.

What're your thoughts?

Cheers,
Phillip

@quanah
Copy link
Owner

quanah commented May 5, 2015

Hm, this seems somewhat broken overall. add/add_s are part of the deprecated OpenLDAP API, and no longer available. The new fuctions are ldap_add_ext and ldap_add_ext_s, which both take dn, attrs, sctrls, cctrls. (I know the Net::LDAPapi code uses them behind the scenes). Overall, I'm fine with the following:

If we move to Net::LDAPapi version 3.1.x, we can change the API however we want. The original concept was to have parity with the underlying C API. Both Mozilla and OpenLDAP's C API's support add_ext(_s). I'd like to see the Net::LDAPapi code remove the deprecated functions, and then we can fix the function documentation to match the correct current C API functions.

@phillipod
Copy link
Collaborator Author

Sounds good to me.

I believe this has been captured sufficiently within #14 and #17 that this particular ticket can be closed off now.

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

No branches or pull requests

2 participants