Skip to content

Where people and agents ship software together. The open-source git platform for the whole job: issues, agents, checks and deploys to the edge.

Commit

A review that only approves or requests changes is one line in the conversation, and code owners whose approval is not required are not shown as something merging waits for. An approval or a request for changes with no text showed as a full comment card with an empty body and an Edit button; on issues and pull requests alike it is now a row on the conversation's rail, the same weight as an event: the verdict's icon on the rail, the reviewer's avatar and name (an agent by its name with its Agent badge, and whom it acted for), "approved these changes" or "requested changes" in the verdict's colour, its Advisory chip when an agent gave it, and when. It has no Edit or Delete, since there is nothing to edit and a verdict is not deleted. A review with text, or one on a line of a file, keeps its card. Events now show an agent author the same way comments do, by its name with the Agent badge, never as a person. In the pull request's Code owners box, a rule nobody approved yet showed an orange hourglass "Waiting for 1 more approval" even when the box's own badge said Approval not required; when the ruleset does not require code owners' approval it now says "No approval yet" in muted text (or who approved), changes requested still shows in red, and approved rules show as before. When approval is required nothing changes.

Checked:
the site's tsc against this worktree's contracts (clean); the site's tests, 921 of 927 passing, the 5 failures (ai-providers, apps, connectors) coming from the main checkout's uncommitted contracts changes that the shared node_modules resolve to, none touching these files.
syntaqxcommitted Parent22641e9Browse files
2 files+102−220/2 viewed
+25−5
219219 );
220220 }
221221
222−/** Where one rule's review stands, in a few words. */
223−function ReviewStatus({ review }: { review: OwnerReview }) {
222+/**
223+ * Where one rule's review stands, in a few words. When the ruleset does not
224+ * require code owners' approval (`required` false), a rule nobody approved
225+ * yet is said plainly, not as something merging waits for.
226+ */
227+function ReviewStatus({ review, required }: { review: OwnerReview; required: boolean }) {
224228 const names = (list: string[]) => list.join(", ");
225229 if (review.changes_requested_by.length > 0) {
226230 return (
246250 </span>
247251 );
248252 }
253+ if (!required) {
254+ return (
255+ <span className="text-xs text-faint">
256+ {review.approved_by.length > 0 ? `Approved by ${names(review.approved_by)}` : "No approval yet"}
257+ </span>
258+ );
259+ }
249260 const more = Math.max(review.required - review.approved_by.length, 1);
250261 return (
251262 <span className="flex items-center gap-1 text-xs text-warn">
257268 }
258269
259270 /** One rule: its section, pattern and line, its owners, the files it covers, and where its review stands. */
260−function OwnerReviewRow({ review, fileHref }: { review: OwnerReview; fileHref: (line: number) => string }) {
271+function OwnerReviewRow({
272+ review,
273+ required,
274+ fileHref,
275+}: {
276+ review: OwnerReview;
277+ /** Whether the ruleset requires code owners' approval to merge. */
278+ required: boolean;
279+ fileHref: (line: number) => string;
280+}) {
261281 const many = review.files.length > 3;
262282 const files = (
263283 <ul className="mt-1 space-y-0.5">
277297 line {review.line}
278298 </Link>
279299 <span className="ml-auto">
280− <ReviewStatus review={review} />
300+ <ReviewStatus review={review} required={required} />
281301 </span>
282302 </div>
283303 <p className="mt-1 flex flex-wrap items-center gap-x-2 gap-y-0.5 text-xs text-muted">
343363 ) : (
344364 <ul className="divide-y divide-line">
345365 {owners.reviews.map((review) => (
346− <OwnerReviewRow key={`${review.section ?? ""}:${review.line}`} review={review} fileHref={fileHref} />
366+ <OwnerReviewRow key={`${review.section ?? ""}:${review.line}`} review={review} required={owners.required} fileHref={fileHref} />
347367 ))}
348368 </ul>
349369 )}
+77−17
402402 );
403403 }
404404
405−/** Something that happened, as one line on the conversation's rail. */
406−function TimelineEvent({ comment, base, filesUrl }: { comment: Comment; base?: string; filesUrl?: string }) {
405+/**
406+ * One line on the conversation's rail: a mark on the rail (a dot unless
407+ * `mark` says otherwise), who, what and when.
408+ */
409+function TimelineRow({ comment, mark, children }: { comment: Comment; mark?: ReactNode; children: ReactNode }) {
407410 return (
408411 <div className="flex items-center gap-3 text-sm text-muted">
409412 <span className="hidden w-8 shrink-0 justify-center sm:flex">
410− <span className="size-2 rounded-full border border-line-strong bg-bg" />
413+ {mark ?? <span className="size-2 rounded-full border border-line-strong bg-bg" />}
411414 </span>
412415 <span className="flex min-w-0 flex-wrap items-center gap-x-1.5 gap-y-0.5">
413− <Avatar name={comment.author.username} size={16} />
414− <PersonLink name={comment.author.username} className="font-medium text-fg hover:underline" />
415− <span>
416− <WithReferences text={comment.body} base={base} filesUrl={filesUrl} />
417− </span>
416+ {comment.agent ? (
417+ <>
418+ <Avatar name={comment.agent.displayName} agent size={16} />
419+ <AgentName agent={comment.agent} />
420+ </>
421+ ) : (
422+ <>
423+ <Avatar name={comment.author.username} size={16} />
424+ <PersonLink name={comment.author.username} className="font-medium text-fg hover:underline" />
425+ </>
426+ )}
427+ {children}
428+ {comment.actingFor && (
429+ <span className="text-xs text-faint">
430+ on behalf of{" "}
431+ <PersonLink name={comment.actingFor.username} className="hover:text-fg hover:underline">
432+ @{comment.actingFor.username}
433+ </PersonLink>
434+ </span>
435+ )}
418436 <span className="text-faint">
419437 · <TimeAgo at={comment.createdAt} />
420438 </span>
423441 );
424442 }
425443
444+/** Something that happened, as one line on the conversation's rail. */
445+function TimelineEvent({ comment, base, filesUrl }: { comment: Comment; base?: string; filesUrl?: string }) {
446+ return (
447+ <TimelineRow comment={comment}>
448+ <span>
449+ <WithReferences text={comment.body} base={base} filesUrl={filesUrl} />
450+ </span>
451+ </TimelineRow>
452+ );
453+}
454+
455+/** A review's verdict, approved or changes requested, in its own colour. */
456+function Verdict({ verdict }: { verdict: keyof typeof VERDICTS }) {
457+ const { label, style } = VERDICTS[verdict];
458+ return (
459+ <span className={`flex items-center gap-1 font-medium ${style}`}>
460+ {verdict === "approve" ? <CircleCheck size={14} /> : <CircleSlash size={14} />}
461+ {label}
462+ </span>
463+ );
464+}
465+
466+/**
467+ * A review that gave a verdict and said nothing else: one line on the rail,
468+ * not a comment with an empty body. There is nothing to edit, and a
469+ * verdict is not deleted, so it has no actions.
470+ */
471+function TimelineVerdict({ comment, verdict }: { comment: Comment; verdict: keyof typeof VERDICTS }) {
472+ return (
473+ <TimelineRow
474+ comment={comment}
475+ mark={
476+ verdict === "approve" ? (
477+ <CircleCheck size={16} className="text-success" />
478+ ) : (
479+ <CircleSlash size={16} className="text-danger" />
480+ )
481+ }
482+ >
483+ <span className={VERDICTS[verdict].style}>{VERDICTS[verdict].label}</span>
484+ {comment.advisory && <AdvisoryChip />}
485+ </TimelineRow>
486+ );
487+}
488+
426489 /** Edit and Delete, under a comment the viewer may change. */
427490 function CommentActions({
428491 comment,
521584 if (comment.kind === "event") {
522585 return <TimelineEvent key={comment.id} comment={comment} base={base} filesUrl={review?.changesUrl} />;
523586 }
524− const verdict = comment.verdict && VERDICTS[comment.verdict];
587+ const verdict = comment.verdict;
588+ // A verdict with nothing said and no line it is about is one line, not a card.
589+ if (verdict && !comment.body.trim() && !comment.path) {
590+ return <TimelineVerdict key={comment.id} comment={comment} verdict={verdict} />;
591+ }
525592 const can = mayChangeComment(comment, viewerId, canModerate);
526593 return (
527594 <TimelineItem
537604 action={
538605 <>
539606 {verdict ? (
540− <span className={`flex items-center gap-1 font-medium ${verdict.style}`}>
541− {comment.verdict === "approve" ? (
542− <CircleCheck size={14} />
543− ) : (
544− <CircleSlash size={14} />
545− )}
546− {verdict.label}
547− </span>
607+ <Verdict verdict={verdict} />
548608 ) : (
549609 <span>{comment.advisory ? "reviewed" : "commented"}</span>
550610 )}

Comments

Sign in to comment.