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

fix(#3268) Adds missing ldap config parameters managerDn managerPassw… #1031

Merged
merged 1 commit into from
Apr 11, 2019

Conversation

senare
Copy link
Contributor

@senare senare commented Sep 6, 2018

Why didn't it get linked to ?

spinnaker/spinnaker#3268

Oki, I need to enable Ldap so trying to fix whats missing here

First time trying to contrib so some hints and tips be welcome, thanx in advance !

I think this should be easy and straight forward but I am still trying to figure out how to build and test all to verify this, but I am opening the PR early/now anyway to gather some feedback and help

@spinnakerbot
Copy link
Contributor

The following commits need their title changed:

5b6dc90: fix(#3268) Adds missing ldap config parameters managerDn managerPassword groupSearchBase

Please format your commit title into the form:

<type>(<scope>): <subject>, e.g. fix(kubernetes): address NPE in status check

This allows us to easily generate changelogs & determine semantic version numbers when cutting releases. You can read more about commit conventions here.

@senare senare force-pushed the master branch 3 times, most recently from dc56e83 to d10e842 Compare September 6, 2018 10:25
@senare
Copy link
Contributor Author

senare commented Sep 7, 2018

Also related spinnaker/spinnaker#2570 ?

@lwander lwander requested a review from stewchen October 12, 2018 16:27
Copy link
Contributor

@stewchen stewchen left a comment

Choose a reason for hiding this comment

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

Please run make to generate the doc changes. Thanks for the change!

@ajordens
Copy link
Contributor

ajordens commented Feb 1, 2019

Can we get this resolved and merged down @stewchen?

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