-
Notifications
You must be signed in to change notification settings - Fork 47
Improvement/review time pr #56
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
Changes from all commits
dbe6397
dcaef09
2efbadf
77cc8a5
6028c8d
7ae0b5d
beb17e7
ddf58f8
9fc4028
497d285
484b051
a7bf8bc
d68ff53
b5d2c32
43c1efa
0d0804a
d389e82
ba6dd7e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,15 +3,88 @@ | |||||||||||||
| <h3 class="text-heading-3 font-semibold font-secondary pb-3"> | ||||||||||||||
| Review time by pull request size | ||||||||||||||
| </h3> | ||||||||||||||
| <p class="text-body-2 text-neutral-500"> | ||||||||||||||
| <p class="text-body-2 text-neutral-500 mb-6"> | ||||||||||||||
| Active contributor is an individual performing tasks such as commits, | ||||||||||||||
| issues, or pull requests during the selected time period. | ||||||||||||||
| </p> | ||||||||||||||
| <hr> | ||||||||||||||
| <section class="mt-5"> | ||||||||||||||
| <div class="w-full min-h-[380px] my-5"> | ||||||||||||||
| <div | ||||||||||||||
| v-if="status === 'success'" | ||||||||||||||
| class="flex flex-col gap-8 text-neutral-900 text-sm" | ||||||||||||||
| > | ||||||||||||||
| <div | ||||||||||||||
| v-for="item in reviewTimeByPr" | ||||||||||||||
| :key="item.sortId" | ||||||||||||||
| class="flex flex-col gap-2" | ||||||||||||||
| > | ||||||||||||||
| <div class="flex flex-row gap-2"> | ||||||||||||||
| <span>{{ item.lines }} lines</span> | ||||||||||||||
| <span class="text-neutral-400">{{ item.prCount }} pull requests</span> | ||||||||||||||
| </div> | ||||||||||||||
| <div class="pr-4"> | ||||||||||||||
| <lfx-progress-bar | ||||||||||||||
| :values="[item.averageReviewTime / maxValue * 100]" | ||||||||||||||
| :label="`${item.averageReviewTime} ${item.averageReviewTimeUnit}`" | ||||||||||||||
| hide-empty | ||||||||||||||
| /> | ||||||||||||||
| </div> | ||||||||||||||
| </div> | ||||||||||||||
| </div> | ||||||||||||||
| <lfx-spinner v-else /> | ||||||||||||||
| </div> | ||||||||||||||
| </section> | ||||||||||||||
| </lfx-card> | ||||||||||||||
| </template> | ||||||||||||||
|
|
||||||||||||||
| <script setup lang="ts"> | ||||||||||||||
| import LfxCard from "~/components/uikit/card/card.vue"; | ||||||||||||||
| import { useFetch, useRoute } from 'nuxt/app'; | ||||||||||||||
| import { computed, watch } from 'vue'; | ||||||||||||||
| import type { ReviewTimeByPrItem } from './types/review-time-by-pr.types'; | ||||||||||||||
| import LfxCard from '~/components/uikit/card/card.vue'; | ||||||||||||||
| import useToastService from '~/components/uikit/toast/toast.service'; | ||||||||||||||
| import { ToastTypesEnum } from '~/components/uikit/toast/types/toast.types'; | ||||||||||||||
| import LfxSpinner from '~/components/uikit/spinner/spinner.vue'; | ||||||||||||||
| import LfxProgressBar from '~/components/uikit/progress-bar/progress-bar.vue'; | ||||||||||||||
|
|
||||||||||||||
| const props = withDefaults( | ||||||||||||||
| defineProps<{ | ||||||||||||||
| timePeriod?: string; | ||||||||||||||
| }>(), | ||||||||||||||
| { | ||||||||||||||
| timePeriod: '90d' | ||||||||||||||
| } | ||||||||||||||
| ); | ||||||||||||||
|
|
||||||||||||||
| const { showToast } = useToastService(); | ||||||||||||||
|
|
||||||||||||||
| const route = useRoute(); | ||||||||||||||
|
|
||||||||||||||
| const { data, status, error } = useFetch( | ||||||||||||||
| `/api/project/${route.params.slug}/development/review-time-by-pr-size`, | ||||||||||||||
| { | ||||||||||||||
| params: { | ||||||||||||||
| project: route.params.slug, | ||||||||||||||
| repository: route.params.name || '', | ||||||||||||||
| 'time-period': props.timePeriod | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| ); | ||||||||||||||
| const reviewTimeByPr = computed<ReviewTimeByPrItem[]>(() => data.value as ReviewTimeByPrItem[]); | ||||||||||||||
| const maxValue = computed(() => Math.max(...reviewTimeByPr.value.map((item) => item.averageReviewTime))); | ||||||||||||||
|
|
||||||||||||||
| watch(error, (err) => { | ||||||||||||||
| if (err) { | ||||||||||||||
| showToast( | ||||||||||||||
| `Error fetching social mentions: ${error.value?.statusMessage}`, | ||||||||||||||
| ToastTypesEnum.negative, | ||||||||||||||
|
Comment on lines
+79
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix incorrect error message text. The error message mentions "social mentions" which doesn't match this component's purpose of displaying review times by pull request size. Update the message to be contextually correct. showToast(
- `Error fetching social mentions: ${error.value?.statusMessage}`,
+ `Error fetching review time data: ${error.value?.statusMessage}`,
ToastTypesEnum.negative,📝 Committable suggestion
Suggested change
|
||||||||||||||
| undefined, | ||||||||||||||
| 10000 | ||||||||||||||
| ); | ||||||||||||||
| } | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| </script> | ||||||||||||||
|
|
||||||||||||||
| <script lang="ts"> | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| export interface ReviewTimeByPrItem { | ||
| sortId: number; | ||
| lines: string; | ||
| prCount: number; | ||
| averageReviewTime: number; | ||
| averageReviewTimeUnit: string; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| import { reviewTimeByPr } from '~~/server/mocks/review-time-by-pr.mock'; | ||
|
|
||
| /** | ||
| * Frontend expects the data to be in the following format: | ||
| * [ | ||
| * { | ||
| * sortId: number; | ||
| * lines: string; | ||
| * prCount: number; | ||
| * averageReviewTime: number; | ||
| * averageReviewTimeUnit: string; | ||
| * } | ||
| * ] | ||
| */ | ||
| /** | ||
| * Query params: | ||
| * - project: string | ||
| * - repository: string | ||
| * - time-period: string // This is isn't defined yet, but we'll add '90d', '1y', '5y' for now | ||
| */ | ||
| export default defineEventHandler(async () => reviewTimeByPr); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| export const reviewTimeByPr = [ | ||
| { | ||
| sortId: 1, | ||
| lines: '1-9', | ||
| prCount: 1000, | ||
| averageReviewTime: 9.86, | ||
| averageReviewTimeUnit: 'days' | ||
| }, | ||
| { | ||
| sortId: 2, | ||
| lines: '10-59', | ||
| prCount: 1200, | ||
| averageReviewTime: 20.96, | ||
| averageReviewTimeUnit: 'days' | ||
| }, | ||
| { | ||
| sortId: 3, | ||
| lines: '60-99', | ||
| prCount: 1100, | ||
| averageReviewTime: 31.32, | ||
| averageReviewTimeUnit: 'days' | ||
| }, | ||
| { | ||
| sortId: 4, | ||
| lines: '100-499', | ||
| prCount: 1300, | ||
| averageReviewTime: 42.07, | ||
| averageReviewTimeUnit: 'days' | ||
| }, | ||
| { | ||
| sortId: 5, | ||
| lines: '500+', | ||
| prCount: 900, | ||
| averageReviewTime: 55.9, | ||
| averageReviewTimeUnit: 'days' | ||
| } | ||
| ]; |
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.
🛠️ Refactor suggestion
Update description to match component purpose.
The current description discusses "active contributors" which doesn't align with the component's purpose of displaying review time by pull request size. Consider updating the text to describe what this widget actually shows.
📝 Committable suggestion