Skip to content

fixed install links#51

Open
evensure wants to merge 1 commit into
Leantime:masterfrom
evensure:master
Open

fixed install links#51
evensure wants to merge 1 commit into
Leantime:masterfrom
evensure:master

Conversation

@evensure

@evensure evensure commented May 25, 2026

Copy link
Copy Markdown

removed "installation/" from each path as the file is located in installation/ already

@marcelfolaron

Copy link
Copy Markdown
Contributor

Maintainer review (advisory — merge authority stays with @marcelfolaron / @broskees)

Intent: Fixes broken relative links in installation/installation-types.md by dropping the redundant installation/ path prefix, since the file already lives in installation/.

Change requests:

  1. Verify against the docs router's link resolution before merge. The change assumes links resolve relative to the current file's directory:
    • installation/installation-types.md:5-7package-installation.md, docker.md, other-methods.md ✔ (relative-to-file)
      The old installation/installation-types.md-style paths would only work if the site root is the docsify/base root. Please confirm which base the docs site uses (docsify typically resolves relative to the served root, not the file) — if it's root-relative, the correct fix may instead be a leading /installation/.... Merge whichever matches the live site.
  2. Confirm the three target files (package-installation.md, docker.md, other-methods.md) all exist in installation/ so none of the three links 404 after the change.

Acceptance checklist (must pass before merge):

  1. All three links click through correctly on the rendered/served docs site (manual test — open each link).
  2. Link style is consistent with how other in-installation/ pages cross-link (relative-to-file vs root-relative).
  3. CLA signed / green (first-time contributor).

CI status: No automated link-checker gate present in this repo. Treat "all three links resolve on the built site" as the manual test gate — this is the one thing to verify since a wrong base assumption would trade three working-in-one-context links for three broken-in-another.

Recommendation: adding a CI link-check (e.g. lychee / markdown-link-check) to this repo would catch exactly this class of PR automatically going forward.

@marcelfolaron

Copy link
Copy Markdown
Contributor

Maintainer review — status: ready (advisory) · priority: P3 · next action: verify link targets, then maintainer merge · owner: @marcelfolaron

1. Intent: Fixes broken relative links in installation/installation-types.md by removing the redundant installation/ path segment (the file already lives in installation/, so the old links resolved to installation/installation/...).

2. Change requests:

  • Verify before merge (correctness): the three rewritten links at installation/installation-types.md:5-7 assume sibling files exist in the same dir:
    -- [Package based installation](installation/package-installation.md)
    -- [Docker based installation](installation/docker.md)
    -- [Other](installation/other-methods.md)
    +- [Package based installation](package-installation.md)
    +- [Docker based installation](docker.md)
    +- [Other](other-methods.md)
    docker.md definitely exists (it's edited in Fix typo in Docker installation documentation #48). Please confirm package-installation.md and other-methods.md also exist in installation/ — if the actual filenames differ, these links would still 404. The fix direction is correct regardless.
  • No other changes needed; this is the right way to fix the double-prefix.

3. Acceptance checklist (must pass before merge):

  • package-installation.md and other-methods.md confirmed present in installation/
  • Docs build/link-check CI green (a link checker would catch any remaining 404)
  • No behavior change — docs-only, no test required

4. CI status: Not readable through my current tooling — confirm green in the UI. If docs CI includes a link checker, that check passing is the real acceptance gate for this one.

Advisory review only — not an approval. Merge authority stays with @marcelfolaron / @broskees.

@marcelfolaron

Copy link
Copy Markdown
Contributor

Maintainer review (advisory — merge authority stays with @marcelfolaron / @broskees) · first review · full diff read at head 759c3c8 · open since 2026-05-25 (>14 days, flagged for age) · first-time contributor 👋

1. Intent: Fixes broken relative links in installation/installation-types.md — the three list links pointed at installation/package-installation.md, installation/docker.md, installation/other-methods.md, but the file lives in installation/, so the installation/ prefix double-nested the path and 404'd. Strips the redundant prefix so the links resolve as siblings.

2. Change requests (this is a real, correct bug fix — these are confirm-before-merge):

  1. Confirm the three targets exist as siblings (installation/package-installation.md, installation/docker.md, installation/other-methods.md). The fix is right if those files sit alongside installation-types.md in installation/. docker.md definitely does (it's edited in Fix typo in Docker installation documentation #48). Verify package-installation.md and other-methods.md are present at that path so the "fixed" links don't point at a different missing file.
  2. Confirm the docs renderer resolves relative links from the file's own directory (standard for GitBook/Docusaurus/MkDocs). If the site's link base is the repo root rather than the file dir, the original installation/… form would have been correct and this would invert the bug. The contributor's diagnosis (file is already in installation/) strongly implies file-relative resolution — just confirm one link renders/clicks through on the built docs site.

3. Acceptance checklist (must pass before merge):

  1. All three links click through to real pages on the built docs site (docs.leantime.io), not just in the raw markdown — this is the gate for a link fix.
  2. The three target files exist at the sibling paths (CR new documentation draft ready for review #1).
  3. Page renders cleanly.

4. CI status: No code/test surface — docs-only. The one verification that matters is that the links actually resolve on the rendered site (checklist #1); a broken-link fix must be confirmed against the real renderer, not the source. A clean, genuinely-useful fix — worth landing this cycle. Open since May 2026.

Advisory review only — not an approval, and I am not marking this ready or merging. Merge authority stays with @marcelfolaron / @broskees.

@marcelfolaron

Copy link
Copy Markdown
Contributor

Maintainer review (advisory — merge authority stays with @marcelfolaron / @broskees) · flagged in today's sweep as a long-open, never-reviewed PR (opened 2026-05-25, >14 days) from a first-time contributor · reviewed at head 759c3c8

1. Intent: Fixes broken install links — removes the leading installation/ from paths in files that already live under installation/, so the relative links resolve correctly.

2. Change requests (this is a link-correctness fix, so it needs one real verification):

  1. Confirm the links resolve on the rendered site, not just in raw Markdown (the whole point of the PR). The docs.leantime.io build may rewrite paths differently than a raw-file relative path — verify each edited link actually lands on the right page in a built preview, since "correct in the repo tree" and "correct on the rendered site" can differ depending on the docs generator's base-path handling.
  2. Check that no other doc page links into these files using the old installation/installation/...-style path that this fix would now break in the opposite direction (grep the repo for the old forms).

3. Acceptance checklist (must pass before merge):

  1. Every edited link resolves to a live page in a built docs preview (CR new documentation draft ready for review #1) — the merge gate for a link fix.
  2. No inbound links elsewhere depend on the old path (CR Update quick-installation.md #2).
  3. N/A — docs-only, no code/schema/API change.

4. CI status: No readable CI gate. Link fixes are the one "docs" class that can silently regress navigation, so the built-preview check in CR #1 is the gate here rather than a rubber-stamp. Thanks, @evensure.

Advisory only — not an approval. Merge authority stays with @marcelfolaron / @broskees.

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.

2 participants