search

etyp / meteor-accounts-ldap

A Meteor package for logging in and creating users with LDAP credentials
MIT License
39 stars 23 forks source link

I fixed a couple of things #35

Open jpmolinamatute opened 7 years ago

jpmolinamatute commented 7 years ago

Hi !

great package, we are going to start using it in production where I work, I had to clone your repo to fixed an undefined variable error when setting up LDAP_DEFAULTS.searchResultsProfileMap to an array then since I was already in the code I decided to do the following:

run code through a lint. updated code to ES6. broke code into prototype functions. improved some validations in case some variables are undefined or they are different datatype than the one expected now you can add "constant" fields to searchResultsProfileMap. fixed undefined variable. updated README.md

please let me know if you have any questions or concern I'll keep working on this package for a little while

etyp commented 7 years ago

Hey @jpmolinamatute - great to see the linting + es6 style added. I'll take a quick look and let you know any questions/concerns I have.

The main thing I'd like to address is adding some baseline of tests to make sure a refactor like this doesn't break the package for others. The ldapjs repo has some sample tests, do you think you could add some basic ones?

jpmolinamatute commented 7 years ago

sure! I'll do that during the weekend

rpesciotta commented 5 years ago

Hi @jpmolinamatute and @etyp , Great work you guys have been doing. I'm evaluating using this package in production, and since I saw all these improvements were presented, but the PR is still open, I need to ask: have you guys worked further on this?