-
Notifications
You must be signed in to change notification settings - Fork 6
feat: pr card cycle time #70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 9 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/.prettierrc">
<violation number="1" location="apps/web/.prettierrc:4">
P2: `trailingComma` is misspelled and set to an unsupported value; Prettier expects `trailingComma` with values `all|es5|none`, so this setting will be ignored.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/web/.prettierrc`:
- Line 4: The Prettier config has a typo and invalid value: change the object
key "traillingComma" to the correct "trailingComma" and replace the invalid
value "always" with one of Prettier's valid options (e.g., "all", "es5", or
"none"); update the property in the configuration file where "traillingComma"
appears so Prettier picks up the intended setting.
In `@apps/web/src/components/card-pull-request/timeline-pull-request.tsx`:
- Around line 182-183: The JSX guard currently uses a truthy check that filters
out legitimate 0 values for cycleTime; update the conditional in
timeline-pull-request.tsx (the JSX block using cycleTime and isMerged) to use a
nullish check so zero is allowed—e.g., replace the truthy guard with a check
like "cycleTime != null && isMerged" (or explicitly "cycleTime !== null &&
cycleTime !== undefined && isMerged") so 0ms renders while preserving the merged
gate.
- Around line 2-11: The import statement from "@mantine/core" in
timeline-pull-request.tsx currently imports unused symbols (Stack, Paper, Group,
Title) causing lint warnings; remove these unused imports from the named import
list so only used components (Timeline, Text, ThemeIcon, Box, Divider) remain,
and run the linter to verify warnings are cleared.
🧹 Nitpick comments (2)
apps/api/src/app/pull-requests/resolvers/pull-requests.schema.ts (1)
93-94: Consider clarifying the field description.The description states "between the first commit and the time it was merged", but the transformer logic uses the earlier of
firstCommitAtorprCreatedAt. This fallback behavior might be worth documenting for API consumers.📝 Suggested documentation update
- "The duration, in milliseconds, between the first commit and the time it was merged" + "The duration, in milliseconds, between the start of coding (first commit or PR creation, whichever is earlier) and the time it was merged"apps/web/src/components/card-pull-request/timeline-pull-request.tsx (1)
188-191: Decouple cycle-time icon color from stale-draft badge.
getColor(1)is tied to the stale-draft badge, which can surface an error color even for merged PRs. Consider a neutral/dedicated color for cycle time.🎨 Example neutral color
- <IconClock size={20} stroke={1.5} color={getColor(1)} /> + <IconClock + size={20} + stroke={1.5} + color="var(--mantine-color-text)" + />
apps/web/src/components/card-pull-request/timeline-pull-request.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/.prettierrc">
<violation number="1" location="apps/web/.prettierrc:4">
P2: Prettier ignores `traillingComma` because the option is misspelled. Rename it to `trailingComma` so the setting takes effect.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/components/card-pull-request/timeline-pull-request.tsx`:
- Around line 163-165: The "pending for" text is shown for closed PRs because
the JSX only checks isMerged and timeToMerge; update the conditional to also
ensure the PR is still open (e.g., require !isClosed or state === 'OPEN') so
closed-but-unmerged PRs don't show "pending for". Replace the fragment using
isMerged and timeToMerge with a check like: !isMerged && !isClosed &&
timeToMerge (or !isMerged && timeToMerge && state === 'OPEN')—use whichever
existing prop (isClosed or state/status) is present in this component to locate
where to change the conditional.
🧹 Nitpick comments (1)
apps/web/src/components/card-pull-request/timeline-pull-request.tsx (1)
56-61: Consider using a dedicated color for the cycle time icon.
getColor(1)was designed for the "stale draft" badge logic, but it's now also used for the cycle time icon (line 178). Currently it returns the default text color unlessstaleDraft?.variant === "error", which may not be the intended behavior for cycle time display.If cycle time should always use a specific color (e.g., text color or success color), consider either:
- Passing a hardcoded color to the cycle time icon
- Adding explicit level handling for cycle time
Also applies to: 178-178
Summary by CodeRabbit
New Features
Style
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.