~ knotless Search/
↑↓ move↵ open⇧↵ all results

chadtmiller.com/knotless

git that lives in your Atmosphere account

knotless.dev
1 collaborator

knotless 0.13.0 Latest 2 packages, 1 image

1
10

The iOS app comments, reviews, reacts, merges, queues, closes and stars

Merged
Merged chadtmiller.com asked to merge into main from chadtmiller.com/knotless:feature/ios-writes

What changes

Someone signed in can now, from the app: comment on a pull or an issue, approve a pull or ask for changes, react to a pull or an issue, close and reopen, merge a pull, add it to the merge queue or take it out, and star a repository. Opening the inbox marks what it shows as seen.

The thread is laid out as GitHub's app lays out a pull: no card at the top, sections edge to edge, and merging within Status with a button as wide as its row. The merge box says where a pull stands in its own words, not the command line's, and offers the queue only to a pull the queue would take.

Why

Reviewing and merging from a phone is most of why the app exists. Each write is the procedure the pages use, so the rules are the server's: what a pull may do comes from getMergeStatus, and a refusal shows in the server's own words.

How it was checked

On a real iPhone, against a test repository: a comment and a reaction, each read back from the server, and a star on and off. A reaction shows at once, and a write no longer reloads the screen. The thread's layout was compared with GitHub's app, and a UI test covers its header's links. ios/check-models.sh passes.

Not yet run: merging, queueing, leaving the queue, approving, asking for changes, closing and reopening. They use the same call as the writes that were run, with other method names, and the merge box's words and buttons were looked at for a clean, conflicting, failed, queued and ejected pull.

Shortcomings

  • No reactions on comments, no line comments, no editing or deleting.
  • Comments are plain text sent as Markdown, with no pictures.
  • A draft can't be marked ready here.
  • KnotlessUITests doesn't cover writes, as it would write to live repositories.

🤖 Generated with Claude Code

Asked for changes

This adds app writes: comment and review, reactions, close and reopen, merge, queue and leave-queue, star, and marking the inbox seen. It also lays the thread out as sections edge to edge. I checked every method name and input in Client.call callers against the lexicons (createComment, addReaction, removeReaction, setItemState, mergePull, queuePull, leaveQueue, starRepo, unstarRepo, updateSeen). They match.

One thing should change before merging. MergeSection reads getMergeStatus once, and again only when queued flips. A write that reloads the thread, such as your own approval or a comment, leaves it stale. After you approve, the box can keep saying "Review required" with no Merge button until you leave the screen. The same happens when checks finish while the thread is open. Re-ask whenever the thread reloads.

Merging, queueing, leaving the queue, approving, asking for changes, and closing/reopening haven't been run, as your body says. The stale status above sits on exactly that path, so run the approve-then-merge sequence once on the phone before this lands.

I did not read the build output. ios.yml only checks the models, so SwiftUI compile errors would not show up in the checks.

ios/Knotless/Views/Writing.swiftline 224Outdated
.padding(16)
.frame(maxWidth: .infinity, alignment: .leading)
.onChange(of: queue) { doing = nil }
.task(id: queued) {
review.knotless.dev

This task reruns only when queued changes, so status is never read again after a comment, approval, or reload (edits += 1). Key it on something that changes with the thread. For example, pass the reload counter in and use .task(id: [queued, refresh]), or the thread's head and review state.

Approved

The stale merge status I asked about is fixed. MergeSection now takes refresh: edits, and its .task(id: "\(queued) \(refresh)") calls getMergeStatus again each time the thread reloads. So after your own approval, a comment, or a finished check, the box can show Merge. I only read the code. Your body says approving, merging and queueing haven't been run on the phone, and I still think the approve-then-merge sequence deserves one run before this lands. I didn't read the build output.

Merged as c8dcbf2
Part of a stack of 6
  1. The iOS app comments, reviews, reacts, merges, queues, closes and stars Landed as c8dcbf2
  2. main