Skip to content
This repository was archived by the owner on Apr 1, 2025. It is now read-only.

Conversation

SelfDevTV
Copy link

This readme change is a quickfix for people that might run into this nasty little bug #84

This readme change is a quickfix for people that might run into this nasty little bug jsumners#84
@SelfDevTV
Copy link
Author

SelfDevTV commented Nov 19, 2020

I have no idea why the tests are failing. I just changed some lines in the readme.md nothing else.

@jurjendijkstra
Copy link

Me too, no idea why the build fails, but in my humble opinion the Usage alinea is supposed to be short. I would leave that as it was, e.g. with only url,baseDN,username,password. The next chapter (Constructor), where the attributes attribute is explained with even an example is the good spot to mention that 'dn' cannot be left out.


```js
var ActiveDirectory = require('activedirectory2');
var config = { url: 'ldap://dc.domain.com',
Copy link
Owner

Choose a reason for hiding this comment

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

With this change, I'd go ahead and change the whole block to look like:

var config = {
  url: 'foo',
  user: [
    // comment
    'dn'
  ]
}

The current reduced format become illegible with the inlined comments.

@jsumners
Copy link
Owner

I have no idea why the tests are failing. I just changed some lines in the readme.md nothing else.

Are you sure your branch it up to date with master?

@jsumners
Copy link
Owner

jsumners commented Dec 3, 2020

The tests are not passing due to ldapjs/node-ldapjs#686

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants