Skip to content

[WIP] Link to OCIS Web UI docu#169

Open
jnweiger wants to merge 2 commits into
masterfrom
ocis-web-ui-xref
Open

[WIP] Link to OCIS Web UI docu#169
jnweiger wants to merge 2 commits into
masterfrom
ocis-web-ui-xref

Conversation

@jnweiger

Copy link
Copy Markdown
Contributor

Suggest to add links in two places.

todo: adoc syntax and backports ...

@jnweiger jnweiger requested a review from mmattel May 16, 2024 10:29
@mmattel

mmattel commented May 16, 2024

Copy link
Copy Markdown
Contributor

I am not really confident about this as the webui docs is user centric and we do not reference in the ocis webui documentation to the ocis admin docs...
We need to rephrase if we point to the admin docs to make the scope clear.

@DeepDiver1975 DeepDiver1975 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖 Automated review by Claude Code review agent.

Overview

This PR adds links pointing from the ownCloud Web UI docs to the OCIS (Infinite Scale) Web UI documentation in two places:

  • modules/ROOT/pages/index.adoc — adds a sentence linking to xref:owncloud_web:index.adoc and to the Infinite Scale Web UI docs.
  • modules/owncloud_web/pages/index.adoc — adds a bare URL pointing to the same Infinite Scale Web UI page.

It also tightens the existing classic UI sentence (removes a redundant "guide"). Small, focused change (+3 / -1, 2 files).

Note: this PR is explicitly marked [WIP], and the description lists open todos: "adoc syntax and backports ...". So the items below are largely the things the author already flagged as unfinished — treat this as not-yet-ready-to-merge.

Code quality / style

  • The wording cleanup in modules/ROOT/pages/index.adoc ("...ownCloud Server UI] is only available...") reads better than before. Good.
  • The xref:owncloud_web:index.adoc[ownCloud Web] reference is valid: the owncloud_web module exists and index.adoc is present, and this matches the existing convention used in modules/owncloud_web/partials/nav.adoc. Good.

Specific suggestions

  1. Broken external link (high priority). Both new links point to https://doc.owncloud.com/ocis/deployment/webui/webui.html, which currently returns HTTP 404 (verified, including following redirects; the base doc.owncloud.com/ocis/ site itself returns 200, so this is a genuinely missing page, not a transient outage). The whole point of the PR is to link to this page, so the target URL needs to be corrected before merge. Please double-check the correct OCIS Web UI docs path.

  2. Bare URL vs. linked text. In modules/owncloud_web/pages/index.adoc the link is added as a bare URL:

    For more information about owncloud Web on Infinite Scale see https://doc.owncloud.com/ocis/deployment/webui/webui.html
    

    For consistency with the rest of the docs (and the other hunk in this same PR), give it link text, e.g.:

    For more information, see the https://doc.owncloud.com/ocis/.../webui.html[ownCloud Web on Infinite Scale] documentation.
    
  3. Capitalization / typo. "owncloud Web" should be "ownCloud Web" to match the brand casing used everywhere else in these files.

  4. Punctuation. The new sentence in modules/owncloud_web/pages/index.adoc is missing a trailing period.

  5. Comma usage. "available for both, ownCloud Server and ... Infinite Scale" — the comma after "both" is grammatically off. Prefer "available for both ownCloud Server and ... Infinite Scale".

Potential issues / risks

  • Link rot / accuracy (primary risk): as noted, the destination 404s today. Merging as-is would ship two broken links into the published docs.
  • Backports: the description mentions backports as a todo. If this content needs to land on release branches as well, that follow-up is still outstanding.
  • AsciiDoc syntax: the bare-URL form will render as an auto-detected link, so it will not break the build, but it is stylistically inconsistent (per suggestion 2).

Otherwise the change is low-risk in scope. The main blocker is the broken target URL; the rest are minor polish items, most of which the author already anticipated with the WIP/todo note.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants