Conversation
WalkthroughThe updates centralize activity type and platform definitions by replacing hardcoded string values with dynamic enum calls. The contributors module now references activity types via the newly introduced Changes
Possibly related PRs
Suggested reviewers
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
gaspergrom
left a comment
There was a problem hiding this comment.
please fix the lint and typescript
1f0aea6 to
1e8a693
Compare
Signed-off-by: Raúl Santos <4837+borfast@users.noreply.github.com>
9c7ec42 to
240904b
Compare
I fixed it in the last PR of the stack. I had to disable Husky's pre-commit hook because it was messing up my commits. (#118). |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
frontend/types/shared/activity-platforms.ts (1)
3-18: Consider using consistent capitalization for platform namesThe platform names have inconsistent capitalization formats. Some are fully capitalized (ALL), some have title case (Github, Git), and others have mixed cases (GroupsIO). Consider standardizing the format across all platforms.
For example, you could use title case for all platform names:
export enum ActivityPlatforms { ALL = 'all', - GITHUB = 'Github', - GIT = 'Git', - GERRIT = 'Gerrit', - GITLAB = 'Gitlab', - GROUPS_IO = 'GroupsIO', - CONFLUENCE = 'Confluence', - JIRA = 'Jira', - DEVTO = 'Devto', - DISCORD = 'Discord', - DISCOURSE = 'Discourse', - HACKERNEWS = 'Hackernews', - LINKEDIN = 'Linkedin', - REDDIT = 'Reddit', - SLACK = 'Slack', - STACKOVERFLOW = 'Stackoverflow', - TWITTER = 'Twitter' + GITHUB = 'GitHub', + GIT = 'Git', + GERRIT = 'Gerrit', + GITLAB = 'GitLab', + GROUPS_IO = 'GroupsIO', + CONFLUENCE = 'Confluence', + JIRA = 'Jira', + DEVTO = 'DevTo', + DISCORD = 'Discord', + DISCOURSE = 'Discourse', + HACKERNEWS = 'HackerNews', + LINKEDIN = 'LinkedIn', + REDDIT = 'Reddit', + SLACK = 'Slack', + STACKOVERFLOW = 'StackOverflow', + TWITTER = 'Twitter'frontend/types/shared/activity-types.ts (1)
1-87: Add type documentation for the ActivityTypes enumConsider adding JSDoc comments to document this enum, especially since it's a shared contract between frontend and backend. This will help improve code maintainability and understanding.
+/** + * Standardized activity types used across both frontend and backend + * This enum serves as a single source of truth for all activity types in the application + */ export enum ActivityTypes { ALL = 'all',frontend/app/components/modules/project/components/contributors/config/metrics.ts (2)
2-2: Use consistent import formattingThe import statement for ActivityTypes uses double quotes while the other import uses single quotes. Consider using consistent quote style for all imports.
import type { DropdownGroupOptions } from '~/components/uikit/dropdown/types/dropdown.types'; -import {ActivityTypes} from "~~/types/shared/activity-types"; +import { ActivityTypes } from '~~/types/shared/activity-types';
5-5: Use ActivityTypes.ALL for consistencyFor the "All activities" option, consider using the
ActivityTypes.ALLenum value for consistency.- { label: '', items: [{ label: 'All activities', value: 'all' }] }, + { label: '', items: [{ label: 'All activities', value: ActivityTypes.ALL.valueOf() }] },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/app/components/modules/project/components/contributors/config/metrics.ts(16 hunks)frontend/types/shared/activity-platforms.ts(1 hunks)frontend/types/shared/activity-types.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Lint & Typescript Check
frontend/types/shared/activity-platforms.ts
[error] 19-19: ESLint: Newline required at end of file but not found (eol-last)
🔇 Additional comments (1)
frontend/app/components/modules/project/components/contributors/config/metrics.ts (1)
10-10:Details
❓ Verification inconclusive
Verify the consistency of AUTHORED_COMMIT usage
The
AUTHORED_COMMITactivity type is used across multiple platforms (Github, Git, Gitlab). Verify that this is intentional and that the same activity type can be shared across different platforms.Also applies to: 64-64, 186-186
🏁 Script executed:
#!/bin/bash # Check if AUTHORED_COMMIT is used consistently across multiple platforms in the backend code # Search for AUTHORED_COMMIT in backend code rg -A 2 -B 2 "authored-commit" --glob="!frontend/**"Length of output: 52
Action Required: Verify Cross-Platform Consistency of AUTHORED_COMMIT
It appears that outside of the frontend, no direct references to "authored-commit" were found with the executed command. Since the intended consistency of usingActivityTypes.AUTHORED_COMMITacross platforms (GitHub, Git, Gitlab) is critical, please manually verify that the same activity type is intended to be shared across these platforms. Ensure that the usage in the following locations is consistent:
- File:
frontend/app/components/modules/project/components/contributors/config/metrics.ts
- Lines: 10, 64, 186
If the absence of references in the backend is by design, please confirm that this does not lead to any integration issues or mismatch in behavior across different platforms.
frontend/app/components/modules/project/components/contributors/config/metrics.ts
Show resolved
Hide resolved
frontend/app/components/modules/project/components/contributors/config/metrics.ts
Show resolved
Hide resolved
Signed-off-by: Raúl Santos <4837+borfast@users.noreply.github.com>
Signed-off-by: Raúl Santos <4837+borfast@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/app/components/modules/project/components/contributors/config/metrics.ts (2)
2-2: Consider consistent import quotes usage.The import statement uses double quotes (
") while the import on line 1 uses single quotes ('). Consider maintaining consistency across all imports.-import {ActivityTypes} from "~~/types/shared/activity-types"; +import { ActivityTypes } from '~~/types/shared/activity-types';
7-7: Correct platform name capitalizationSome platform names have incorrect capitalization:
- "Github" should be "GitHub"
- "Gitlab" should be "GitLab"
- "Stackoverflow" should be "Stack Overflow"
Consider updating these for accuracy and consistency.
- label: 'Github', + label: 'GitHub',- label: 'Gitlab', + label: 'GitLab',- label: 'Stackoverflow', + label: 'Stack Overflow',Also applies to: 151-151, 354-354
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/app/components/modules/project/components/contributors/config/metrics.ts(16 hunks)frontend/types/shared/activity-platforms.ts(1 hunks)frontend/types/shared/activity-types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/types/shared/activity-platforms.ts
- frontend/types/shared/activity-types.ts
🔇 Additional comments (3)
frontend/app/components/modules/project/components/contributors/config/metrics.ts (3)
113-119: Fix duplicate activity type references in Gerrit sectionYou're using both
CHANGESET_NEWandCHANGESET_CREATEDfor the same label "Created a changeset". This is redundant and one of them should be removed.{ value: ActivityTypes.CHANGESET_NEW.valueOf(), label: 'Created a changeset' }, - { - value: ActivityTypes.CHANGESET_CREATED.valueOf(), - label: 'Created a changeset' - },
2-2: Consider importing ActivityPlatforms enumSince the metrics are organized by platform labels (e.g., 'Github', 'Git', 'Gerrit', etc.), consider also importing and using the
ActivityPlatformsenum for consistency.import type { DropdownGroupOptions } from '~/components/uikit/dropdown/types/dropdown.types'; -import {ActivityTypes} from "~~/types/shared/activity-types"; +import { ActivityTypes } from '~~/types/shared/activity-types'; +import { ActivityPlatforms } from '~~/types/shared/activity-platforms';Then update each platform label to use the enum:
{ - label: 'Github', + label: ActivityPlatforms.GITHUB.valueOf(), items: [
4-378: Good job centralizing activity type definitionsThe replacement of hardcoded string values with enum references is a good improvement that will help maintain consistency between frontend and backend. This change directly addresses the PR objective of establishing a contract for activity types.
Linear ticket.
This PR introduces enum values for the activity types and platforms, so that both the frontend and the backend can use the same values and not risk mismatches like the ones we've been having.
ℹ️ This PR is part of a stack
Please review them in order, as some of the first ones set the foundation for the later ones.
Summary by CodeRabbit
New Features
Refactor