docs: post fixes for oc11#1562
Conversation
Co-authored-by: Phil Davis <[email protected]>
DeepDiver1975
left a comment
There was a problem hiding this comment.
Review: docs post fixes for OC11
Thanks for this — the direction is right. Migrating the config examples from config.php snippets to the container OWNCLOUD_* env-var form matches how OC11 is deployed, and the OIDC section restructuring (demoting the per-IdP examples to === under a single == General Example Setup, and removing the now-obsolete "Save Settings in the Database" section) is a good cleanup.
My main concern is correctness of the new env-var snippets — several of the JSON examples are not valid JSON and would fail if an admin copy-pasted them. Since the whole value of this PR is copy-pasteable configuration, these are worth fixing before merge. Details inline.
Must fix (broken markup / invalid examples)
envvars.adoc—CUSTOMGROUPS_DISALLOWED_GROUPSxref has a misplaced);OPENID_CONNECTxref is missing thexref:prefix.login_policies.adoc— both the template and the example JSON have syntax errors ("allowOnly|reject"as a key, unquoted keys, unescaped backslashes in the OIDC class name, a missing quote, a stray quote).user_auth_ftp_smb_imap.adoc— typoopwnCloud.general_troubleshooting.adoc— grammar "problems raise" → "arise".
Please confirm (uncertain — flagging rather than assuming)
- Value formats. This PR introduces three different multi-value encodings: JSON arrays (
USER_BACKENDS,MEMCACHED_SERVERS), quoted CSV (CUSTOMGROUPS_DISALLOWED_GROUPS), and the login-policy JSON map. Can you confirm each matches exactly what the container entrypoint parses? A wrong format here is worse than the oldconfig.phpdocs because it looks authoritative. - Multi-line
.envvalues. The login-policy example uses trailing\line continuations inside a[source,.env]block — does the entrypoint actually accept that? - Troubleshooting content removal. The Windows WebDAV
0x80070043Apache workaround and the CalDAV/CardDAV service-discovery guidance are being replaced with "open an issue." That's still-valid, non-OC11-specific admin content — is removing it (vs. updating it) intentional?
Nits
- xref anchor
#define-the-user_backends-app— verify it resolves given the backtick-formatted target heading. caching_configuration.adoc— a one-line note on whatweightmeans would help.
No test coverage concerns (docs-only). Overall a solid update once the invalid snippets are corrected.
| ], | ||
| OWNCLOUD_USER_BACKENDS={"class":..., "arguments":[...]} | ||
| ---- | ||
|
|
There was a problem hiding this comment.
The xref target #define-the-user_backends-app — the heading in config_sample_php_parameters.adoc is === Define the \user_backends` app. With the backtick-wrapped user_backends, Antora's auto-generated ID may not be define-the-user_backends-app(backticks/formatting can affect the generated anchor). Please verify this resolves during a build, or add an explicit[#define-the-user_backends-app]` anchor on the target heading.
| // Hide the Custom Groups app for users in the | ||
| // 'no_guest_app_users' and 'project5' group. | ||
| 'customgroups.disallowed-groups' => ['no_guest_app_users', 'project5'], | ||
| OWNCLOUD_CUSTOMGROUPS_DISALLOWED_GROUPS='no_guest_app_users','project5' |
There was a problem hiding this comment.
Format consistency: this example uses OWNCLOUD_CUSTOMGROUPS_DISALLOWED_GROUPS='no_guest_app_users','project5' (single-quoted, comma-separated), while the corresponding envvars.adoc entry describes it as a plain "comma-separated list of group names" and the other new multi-value envvars in this PR use JSON arrays. Can you confirm the container actually expects this quoted-CSV form (and not e.g. no_guest_app_users,project5 or a JSON array)? The three formats introduced across this PR (JSON arrays, quoted CSV, plain CSV) should each match what the entrypoint parses.
There was a problem hiding this comment.
Intentionally, a reminder to the admin to avoid parsing special characters wrong
There are also syntax errors to be fixed.
This PR provides some post fixes related to the OC11 docs.
RemovalUpdate of oidc configuration(needs mentioning in the release notes)Note that memcached changes will be added as soon as the fixes are available. Keeping the PR as draft therefore.