Link the install guide from the apt section - #46
guys-inc-ops[bot] wants to merge 1 commit into
Conversation
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WvZtGGgfkaA6yNRwAD9rX2
![clippy-qa[bot]](https://avatars.githubusercontent.com/in/4024528?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clippy reviewed this pull request
The diff is a single documentation line added to README.md linking to an external install guide. All three surviving findings are in that one line: an unverifiable content claim, a maintenance divergence risk from duplicating instructions already present inline, and a link-text style inconsistency. No code is touched.
3 findings — 2 blocking · 1 nit · 0 pre-existing
Requesting changes — there are findings to address. Address each one and mark its thread Resolved (reply first if you think a finding is wrong — a resolved thread is a decision, not a dismissal). Push, or comment /review, and I will review the new commits.
clippy-review: verdict=changes event=REQUEST_CHANGES normal=2 nit=1 pre_existing=0 inline=3 body=0 unresolved=0 head=df3466e
| sudo apt update && sudo apt install github-desktop | ||
| ``` | ||
|
|
||
| The same steps with checksums for direct downloads, plus `.rpm` and AppImage instructions, are on the install guide at [guysinc.pub/install.html](https://guysinc.pub/install.html). |
There was a problem hiding this comment.
Checksum claim references an external page that cannot be verified from repo contents
The new sentence promises 'checksums for direct downloads' at guysinc.pub/install.html. The only guysinc.pub HTML file in the repo (apt/index.html) contains no checksums at all. The linked page is external and untracked, so there is no way to confirm the claim is accurate — if the install guide does not actually list checksums, the README misleads security-conscious users who rely on that verification step.
| sudo apt update && sudo apt install github-desktop | ||
| ``` | ||
|
|
||
| The same steps with checksums for direct downloads, plus `.rpm` and AppImage instructions, are on the install guide at [guysinc.pub/install.html](https://guysinc.pub/install.html). |
There was a problem hiding this comment.
External link duplicates .rpm and AppImage instructions already present inline in the README
The sentence points readers to guysinc.pub/install.html for '.rpm and AppImage instructions', but those instructions are already written out in full at README.md lines 90–104. This creates two authoritative sources for the same content. When package names, flags, or steps change, the inline README section and the external page can independently drift, leaving one set of instructions stale with no diff to catch it.
| sudo apt update && sudo apt install github-desktop | ||
| ``` | ||
|
|
||
| The same steps with checksums for direct downloads, plus `.rpm` and AppImage instructions, are on the install guide at [guysinc.pub/install.html](https://guysinc.pub/install.html). |
There was a problem hiding this comment.
nit: Bare URL used as Markdown anchor text, inconsistent with every other link in the file
The new link is [guysinc.pub/install.html](https://guysinc.pub/install.html) — a bare domain as the display text. Every other Markdown link in README.md uses descriptive anchor text: [Releases](...), [Known issues](...), [GitHub Desktop](...), etc. Bare URLs inside code blocks (the apt shell commands on lines 55–57) are command arguments, not anchors, so they do not establish this pattern. The new link is the sole inconsistency.
There was a problem hiding this comment.
Clippy reviewed this pull request
The diff adds a single documentation sentence in README.md. No code logic is changed. Three confirmed issues were found in the new sentence: the referent "the same steps" is factually misleading (the apt block sets up a repository, not direct downloads), the link display text is a bare URL unlike every other link in the file, and AppImage is written inconsistently with the rest of the document. A fourth structural issue (link missing from the "Other distributions" section where it is most relevant) is plausible.
4 findings — 1 blocking · 3 nit · 0 pre-existing
1 finding could not be anchored to the diff, so they are here instead:
README.md:84— nit: Install guide link absent from "Other distributions" section, the natural entry point for .rpm and AppImage users The link to the install guide appears only in the Debian/Ubuntu section (line 60), but the guide is described as covering .rpm and AppImage — formats documented in "Other distributions" (lines 84–104). A user who skips the apt section to go straight to "Other distributions" never sees the pointer. Adding the same link at the start of that section would serve the intended audience.
Requesting changes — there are findings to address. Address each one and mark its thread Resolved (reply first if you think a finding is wrong — a resolved thread is a decision, not a dismissal). Push, or comment /review, and I will review the new commits.
clippy-review: verdict=changes event=REQUEST_CHANGES normal=1 nit=3 pre_existing=0 inline=3 body=1 unresolved=0 head=df3466e
| sudo apt update && sudo apt install github-desktop | ||
| ``` | ||
|
|
||
| The same steps with checksums for direct downloads, plus `.rpm` and AppImage instructions, are on the install guide at [guysinc.pub/install.html](https://guysinc.pub/install.html). |
There was a problem hiding this comment.
"The same steps" contradicts "direct downloads" — the apt block sets up a repository, not a direct download
The new sentence says "The same steps with checksums for direct downloads…" but the four lines above (lines 53–58) configure a signed apt repository — they are not direct-download steps. A direct download would mean fetching a .deb file from the Releases page and verifying its checksum; the apt commands do neither. A reader following the install guide expecting "the same steps" as above would be confused. The sentence needs to separate the two ideas: the guide covers (a) direct .deb download with checksum verification and (b) .rpm/AppImage instructions — neither of which is "the same steps" as the apt repository setup.
| sudo apt update && sudo apt install github-desktop | ||
| ``` | ||
|
|
||
| The same steps with checksums for direct downloads, plus `.rpm` and AppImage instructions, are on the install guide at [guysinc.pub/install.html](https://guysinc.pub/install.html). |
There was a problem hiding this comment.
nit: Link display text is a bare URL, inconsistent with every other hyperlink in the file
guysinc.pub/install.html uses the raw domain path as display text. Every other link in README.md uses descriptive anchor text: "Releases", "Arch Wiki", "roadmap", "Known issues", "README". A consistent fix is install guide, which also makes the sentence read more naturally and removes the redundancy of showing the URL twice.
| sudo apt update && sudo apt install github-desktop | ||
| ``` | ||
|
|
||
| The same steps with checksums for direct downloads, plus `.rpm` and AppImage instructions, are on the install guide at [guysinc.pub/install.html](https://guysinc.pub/install.html). |
There was a problem hiding this comment.
nit: AppImage written without leading dot, inconsistent with .AppImage used throughout the file
Line 60 writes AppImage (bare word, no dot, no backticks). The rest of the file uses .AppImage with the leading dot consistently: in the format list (line 45), in the "Other distributions" intro (line 88), and in the install command (line 102). The fix is to write .AppImage to match the style of .rpm in the same sentence.
There was a problem hiding this comment.
Clippy reviewed this pull request
The diff adds one sentence to README.md pointing readers to an external install guide. Three real issues were found: the URL uses the bare domain while every other reference in the codebase uses the apt. subdomain (likely a broken link), the sentence implies RPM and AppImage instructions exist only externally when the README already contains them inline, and docs/installation.md was not updated to reference the guide it now implicitly supersedes.
4 findings — 2 blocking · 2 nit · 0 pre-existing
1 finding could not be anchored to the diff, so they are here instead:
docs/installation.md:17— nit: docs/installation.md lists Linux formats but gives no install commands and no pointer to the new install guide README.md now directs readers to guysinc.pub/install.html as the authoritative guide for checksums and package formats, but docs/installation.md still lists AppImage, Debian, and RPM as available Linux formats with no install commands and no reference to the guide. A user reading the docs folder for Linux install instructions hits a dead end while README.md users get a cross-link. Updating docs/installation.md to at least reference the install guide URL would keep the two entry points consistent.
Requesting changes — there are findings to address. Address each one and mark its thread Resolved (reply first if you think a finding is wrong — a resolved thread is a decision, not a dismissal). Push, or comment /review, and I will review the new commits.
clippy-review: verdict=changes event=REQUEST_CHANGES normal=2 nit=2 pre_existing=0 inline=3 body=1 unresolved=0 head=df3466e
| sudo apt update && sudo apt install github-desktop | ||
| ``` | ||
|
|
||
| The same steps with checksums for direct downloads, plus `.rpm` and AppImage instructions, are on the install guide at [guysinc.pub/install.html](https://guysinc.pub/install.html). |
There was a problem hiding this comment.
Link uses bare domain; all other repo URLs use apt. subdomain
The added link points to https://guysinc.pub/install.html, but every other guysinc.pub URL in the repository uses https://apt.guysinc.pub/… (e.g. lines 55–56 use apt.guysinc.pub/guysinc-apt.gpg and apt.guysinc.pub/github-desktop). No other file in the repo references the bare guysinc.pub domain. The intended URL is almost certainly https://apt.guysinc.pub/install.html; as written the link likely 404s.
| sudo apt update && sudo apt install github-desktop | ||
| ``` | ||
|
|
||
| The same steps with checksums for direct downloads, plus `.rpm` and AppImage instructions, are on the install guide at [guysinc.pub/install.html](https://guysinc.pub/install.html). |
There was a problem hiding this comment.
Sentence implies RPM/AppImage instructions exist only externally; they are already in this file
The sentence says .rpm and AppImage instructions are on the install guide at an external link, which implies a reader must leave the README to find those instructions. The README already contains inline RPM install commands (around lines 90–97) and AppImage instructions (around lines 99–104) under the Other distributions section. A reader following the external link pointer will either miss the in-repo instructions entirely or later distrust the README's completeness. The sentence should either remove the RPM/AppImage mention or clarify that the README also covers them.
| sudo apt update && sudo apt install github-desktop | ||
| ``` | ||
|
|
||
| The same steps with checksums for direct downloads, plus `.rpm` and AppImage instructions, are on the install guide at [guysinc.pub/install.html](https://guysinc.pub/install.html). |
There was a problem hiding this comment.
nit: Sentence interrupts the logical flow between the APT code block and its GPG trust explanation
Lines 55–58 show the APT repository setup commands; lines 62–68 explain why the repository is safe to trust (GPG fingerprint). The new sentence at line 60 redirects readers to an external page, creating a detour between the commands and their security context. A reader who follows the external link and returns may lose the thread and skip GPG verification. Moving the sentence to after line 63, or to the Other distributions section, would preserve the security-explanation flow.
One line under the apt block pointing at guysinc.pub/install.html, which carries the same steps plus checksums, .rpm and AppImage. The page has no external reference today; this README is crawled.
🤖 Generated with Claude Code
https://claude.ai/code/session_01WvZtGGgfkaA6yNRwAD9rX2