Note in Set-remote that saving creates a new package revision - #767
Conversation
|
@greptileai The revision-notice overstatement is fixed in 89aa2ce — instead of weakening the copy to "may", the notice is now gated on the package actually having a local commit ( |
|
|
||
| // Only the editable (first-push) path re-commits on save; | ||
| // the locked "Show remote" view creates nothing. | ||
| {(!locked).then(|| view! { |
There was a problem hiding this comment.
Notice Still Overstates
This notice still renders for every editable Set-remote dialog. A package with no local commit is editable, but saving in that state only writes the remote into lineage and skips the recommit path, so no new revision is created. Users can still be told that Save will create a revision when it will only record the remote.
There was a problem hiding this comment.
This re-review ran before the fix commit reached the remote — my re-review request went out at 16:31 but 89aa2ce wasn't pushed until just after, so the branch you re-read was still the pre-fix code. 89aa2ce is now on origin: the notice renders under (!locked && has_local_commit) (set_remote_popup.rs:365), and has_local_commit = lineage.commit.is_some() — the exact condition set_remote gates the recommit on — threaded from both backends. A commit-less editable dialog now shows no notice. Please re-review the current head.
There was a problem hiding this comment.
You're right — I reviewed stale code. Looking at the current head (89aa2ce), the condition at line 365 is (!locked && has_local_commit), and the prop doc above it explicitly states that has_local_commit mirrors lineage.commit.is_some() — the same predicate the backend gates the recommit on. A commit-less editable dialog correctly shows no notice.
My original concern is fully addressed. This is good to merge.
Summary
The Set-remote dialog looks like it just records an address, but saving it re-commits the package (rehashed for the bucket, workflow stamped and validated) — a new revision. Users had no way to know that, which is also why the workflow selector in the dialog felt unexplained.
Adds a short semi-bold line between the workflow selector and the Save/Cancel buttons:
That framing makes the operation's nature clear (it's a commit-like action, hence the workflow choice) without exposing internals. Shown only in the editable first-push mode — the locked "Show remote" view creates nothing.
Deliberately says "new revision," not "new commit hash": the hash value isn't actionable and surfacing raw hashes to non-engineers was flagged as confusing in earlier user feedback; "revision" also implies something local (nothing is uploaded until push).
Testing
Pure UI copy + one CSS rule. wasm clippy clean; UI tests pass.
Versions
quilt-sync
0.18.3-alpha14(alpha13 is taken by the open PR #766; renumber if these merge out of order).Greptile Summary
This PR clarifies the Set-remote dialog and updates the release metadata.
0.18.3-alpha14.Confidence Score: 4/5
This is close, but the dialog copy should be fixed before merging.
quilt-sync/ui/src/components/set_remote_popup.rs
Important Files Changed
Reviews (2): Last reviewed commit: "Note in Set-remote that saving creates a..." | Re-trigger Greptile