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

occ LDAP update #4300

Merged
merged 2 commits into from
Dec 2, 2021
Merged

occ LDAP update #4300

merged 2 commits into from
Dec 2, 2021

Conversation

mmattel
Copy link
Contributor

@mmattel mmattel commented Nov 18, 2021

This PR is a bigger update of the occ ldap command set as it was a complete mess.

Fixes: #3901 (New LDAP command to invalidate the cache)

Merging when owncloud/user_ldap#681 (LDAP 0.16.0) is close to final.

When merged, the occ ldap command reference can safely be included in the ldap documentation when it gets revised.

Backport to 10.9 and 10.8

@jvillafanez fyi

Copy link
Contributor

@EParzefall EParzefall left a comment

Choose a reason for hiding this comment

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

Minor language improvements.

@mmattel mmattel force-pushed the update_occ_ldap branch 2 times, most recently from 5d09215 to 7d87586 Compare November 23, 2021 11:04
Copy link
Contributor

@jnweiger jnweiger left a comment

Choose a reason for hiding this comment

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

Some added details and precision.

@mmattel mmattel force-pushed the update_occ_ldap branch 2 times, most recently from ae541e7 to 7205cfa Compare November 28, 2021 13:07
@mmattel
Copy link
Contributor Author

mmattel commented Nov 30, 2021

@jvillafanez info: the texts from the options and arguments are not freely written but come when you use --help and are printed to the screen. I am just copying that... In case you need a change, pls change the texts in the app - where I guess it is still time before a new release is out.

@jvillafanez
Copy link
Member

Ok, got it. The first one (the search argument) has a different help message in the release: just "the search string (can be empty)", which seems lacking. That's why I thought the documentation would include some additional information. We can't write a wall of text in a help message.

For the second one, I don't think it's critical, so we can leave it as it is.

@mmattel
Copy link
Contributor Author

mmattel commented Nov 30, 2021

That's why I thought the documentation would include some additional information.

Will do my very best (Dinner for One) 😅

@mmattel mmattel marked this pull request as ready for review November 30, 2021 13:50
@mmattel
Copy link
Contributor Author

mmattel commented Nov 30, 2021

@jvillafanez mind to have a look at the search changes if they are ok to you (pls try to reword in case they are not...)

@mmattel mmattel force-pushed the update_occ_ldap branch 2 times, most recently from f190e58 to c40a771 Compare December 1, 2021 12:13
@mmattel
Copy link
Contributor Author

mmattel commented Dec 1, 2021

It needs more clarification text with regards of multiple values when using ldap:set-config.
Will work on it asap (in this PR).

@mmattel
Copy link
Contributor Author

mmattel commented Dec 2, 2021

The document is updated and contains now all changes and explanations.
You can view a rendered version here: https://github.com/owncloud/docs/blob/5b9717ec42666600fd41b74368c71fd16db135c1/modules/admin_manual/pages/configuration/server/occ_commands/app_commands/_ldap_integration_commands.adoc

@jvillafanez mind to have a final look on it before we merge?

@mmattel mmattel merged commit f9e3b3d into master Dec 2, 2021
@delete-merged-branch delete-merged-branch bot deleted the update_occ_ldap branch December 2, 2021 19:40
mmattel added a commit that referenced this pull request Dec 2, 2021
mmattel added a commit that referenced this pull request Dec 2, 2021
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.

New LDAP command to invalidate the cache
4 participants