-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fixed incorrect imports #8654
Fixed incorrect imports #8654
Conversation
WalkthroughThe changes in this pull request primarily involve updating import statements across multiple files in the CVAT UI codebase. Specifically, the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PreviewComponent
participant ProjectTopBar
participant ProjectListComponent
User->>PreviewComponent: Request preview
PreviewComponent->>cvat-core-wrapper: Import Job, Task, Project
PreviewComponent->>User: Display preview
User->>ProjectTopBar: Navigate to project
ProjectTopBar->>cvat-core-wrapper: Import Project
ProjectTopBar->>User: Display project top bar
User->>ProjectListComponent: View project list
ProjectListComponent->>cvat-core-wrapper: Import Project
ProjectListComponent->>User: Display project list
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
Quality Gate passedIssues Measures |
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: 0
🧹 Outside diff range and nitpick comments (1)
cvat-ui/src/components/project-page/top-bar.tsx (1)
Line range hint
1-52
: Consider updating PR checklist.The PR checklist indicates that tests are not added/updated. While this change is type-level only, it would be good to ensure type checking tests are in place to prevent future regressions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
cvat-ui/src/components/common/preview.tsx
(1 hunks)cvat-ui/src/components/project-page/top-bar.tsx
(1 hunks)cvat-ui/src/components/projects-page/project-list.tsx
(1 hunks)
🔇 Additional comments (3)
cvat-ui/src/components/project-page/top-bar.tsx (1)
14-14
: LGTM! Verify type availability.
The change to import Project
from 'cvat-core-wrapper' instead of 'reducers' is architecturally sound, as core types should be defined in the core wrapper.
Let's verify the type exists and is properly exported:
cvat-ui/src/components/projects-page/project-list.tsx (1)
12-13
: LGTM! Import reorganization looks good.
The change to import Project
type from 'cvat-core-wrapper' instead of 'reducers' is appropriate and aligns with the codebase restructuring. This helps maintain better separation of concerns by sourcing type definitions from a dedicated core wrapper module rather than the Redux reducers.
Let's verify this change is consistent across the codebase:
✅ Verification successful
Project type imports need attention in two files
The verification reveals inconsistency in Project-related imports. While most files correctly import Project
from 'cvat-core-wrapper', there are two files still using ProjectsQuery
from 'reducers':
cvat-ui/src/components/projects-page/top-bar.tsx
cvat-ui/src/components/projects-page/projects-page.tsx
Import changes are correct - ProjectsQuery is a different type
The verification shows that ProjectsQuery
is a distinct interface used for query parameters and filtering, completely different from the Project
entity type. It's correctly imported from 'reducers' as it's part of the Redux state management. The current import changes in project-list.tsx
are appropriate because:
Project
type represents the core entity and is correctly imported from 'cvat-core-wrapper'ProjectsQuery
in other files is a separate interface for filtering/pagination, defined in reducers
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Project type import consistency
# Expected: All Project type imports should be from cvat-core-wrapper
# Check for any remaining Project imports from reducers
echo "Checking for any remaining Project imports from reducers..."
rg "import.*\{.*Project.*\}.*from.*['\"]reducers['\"]"
# Check for new Project imports from cvat-core-wrapper
echo "Checking new Project imports from cvat-core-wrapper..."
rg "import.*\{.*Project.*\}.*from.*['\"]cvat-core-wrapper['\"]"
Length of output: 1399
Script:
#!/bin/bash
# Let's check the usage context of ProjectsQuery to understand if it's a different type
echo "Checking ProjectsQuery definition and usage..."
rg "ProjectsQuery" -A 3 -B 3
# Also check if there are any other Project-related types in reducers
echo "Checking for Project-related type definitions in reducers..."
ast-grep --pattern 'type $_ = { $$$ Project $$$ }'
Length of output: 6271
cvat-ui/src/components/common/preview.tsx (1)
14-15
: Verify type compatibility with cvat-core-wrapper.
The migration of types from reducers
to cvat-core-wrapper
looks good and aligns with better architectural practices by centralizing type definitions. However, we should ensure type compatibility is maintained.
Let's verify the type definitions and their usage:
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #8654 +/- ##
===========================================
- Coverage 74.39% 74.39% -0.01%
===========================================
Files 401 401
Lines 43421 43421
Branches 3951 3951
===========================================
- Hits 32303 32302 -1
- Misses 11118 11119 +1
|
Motivation and context
How has this been tested?
Checklist
develop
branch(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
Job
,Task
, andProject
to source them fromcvat-core-wrapper
across multiple components, ensuring consistency in type definitions.