Merge branch 'worktree-agent-a501bb99f3d43b29a' into ship-fixes
2 files+102−220/2 viewed
| 219 | 219 | ); | |
| 220 | 220 | } | |
| 221 | 221 | ||
| 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 }) { | |
| 224 | 228 | const names = (list: string[]) => list.join(", "); | |
| 225 | 229 | if (review.changes_requested_by.length > 0) { | |
| 226 | 230 | return ( | |
| ⋯ | |||
| 246 | 250 | </span> | |
| 247 | 251 | ); | |
| 248 | 252 | } | |
| 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 | + | } | |
| 249 | 260 | const more = Math.max(review.required - review.approved_by.length, 1); | |
| 250 | 261 | return ( | |
| 251 | 262 | <span className="flex items-center gap-1 text-xs text-warn"> | |
| ⋯ | |||
| 257 | 268 | } | |
| 258 | 269 | ||
| 259 | 270 | /** 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 | + | }) { | |
| 261 | 281 | const many = review.files.length > 3; | |
| 262 | 282 | const files = ( | |
| 263 | 283 | <ul className="mt-1 space-y-0.5"> | |
| ⋯ | |||
| 277 | 297 | line {review.line} | |
| 278 | 298 | </Link> | |
| 279 | 299 | <span className="ml-auto"> | |
| 280 | − | <ReviewStatus review={review} /> | |
| 300 | + | <ReviewStatus review={review} required={required} /> | |
| 281 | 301 | </span> | |
| 282 | 302 | </div> | |
| 283 | 303 | <p className="mt-1 flex flex-wrap items-center gap-x-2 gap-y-0.5 text-xs text-muted"> | |
| ⋯ | |||
| 343 | 363 | ) : ( | |
| 344 | 364 | <ul className="divide-y divide-line"> | |
| 345 | 365 | {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} /> | |
| 347 | 367 | ))} | |
| 348 | 368 | </ul> | |
| 349 | 369 | )} | |
| 402 | 402 | ); | |
| 403 | 403 | } | |
| 404 | 404 | ||
| 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 }) { | |
| 407 | 410 | return ( | |
| 408 | 411 | <div className="flex items-center gap-3 text-sm text-muted"> | |
| 409 | 412 | <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" />} | |
| 411 | 414 | </span> | |
| 412 | 415 | <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 | + | )} | |
| 418 | 436 | <span className="text-faint"> | |
| 419 | 437 | · <TimeAgo at={comment.createdAt} /> | |
| 420 | 438 | </span> | |
| ⋯ | |||
| 423 | 441 | ); | |
| 424 | 442 | } | |
| 425 | 443 | ||
| 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 | + | ||
| 426 | 489 | /** Edit and Delete, under a comment the viewer may change. */ | |
| 427 | 490 | function CommentActions({ | |
| 428 | 491 | comment, | |
| ⋯ | |||
| 521 | 584 | if (comment.kind === "event") { | |
| 522 | 585 | return <TimelineEvent key={comment.id} comment={comment} base={base} filesUrl={review?.changesUrl} />; | |
| 523 | 586 | } | |
| 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 | + | } | |
| 525 | 592 | const can = mayChangeComment(comment, viewerId, canModerate); | |
| 526 | 593 | return ( | |
| 527 | 594 | <TimelineItem | |
| ⋯ | |||
| 537 | 604 | action={ | |
| 538 | 605 | <> | |
| 539 | 606 | {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} /> | |
| 548 | 608 | ) : ( | |
| 549 | 609 | <span>{comment.advisory ? "reviewed" : "commented"}</span> | |
| 550 | 610 | )} | |
Comments
Sign in to comment.