-
Notifications
You must be signed in to change notification settings - Fork 4
[LFXV2-911] Remove unused indexer tags #37
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Andres Tobon <[email protected]>
WalkthroughRemoved several unprefixed tag emissions from project model tag methods, updated tests to expect prefixed tag forms, bumped GoA CLI version/path in the Makefile, and incremented the Helm chart version. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (2)**/*_test.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧬 Code graph analysis (1)internal/domain/models/project_test.go (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (3)
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/domain/models/project.go (2)
61-85: Update tests to reflect new tag format - tests are currently out of sync with implementation.The Tags() method now generates only prefixed tags (project_uid:, project_slug:, project_name:), but test expectations in project_test.go still require unprefixed tags, Description, and parent_uid tags. Lines 76-84, 92-96, and 120-128 in project_test.go must be updated to match the current implementation.
Additionally:
- No code was found querying by the removed unprefixed tag formats, which is positive for backwards compatibility
- parent_uid remains in the ProjectBase struct but is no longer included in Tags()—confirm this is intentional
- README.md documentation mentioned in the comment (line 59) was not updated in this PR
90-104: ProjectSettings.Tags() is missing MissionStatement tag output; current implementation breaks test expectations.The Tags() method now only returns the project_uid tag. However, test cases in internal/domain/models/project_test.go explicitly expect MissionStatement to be included in the returned tags:
- Test "settings with MissionStatement only" expects: ["Our mission is to test"]
- Test "settings with all tag fields" expects: ["settings-123", "project_uid:settings-123", "Our mission is to test"]
- Test "settings with all fields including non-tag fields" expects: the same
The current implementation violates these test expectations and will cause them to fail. Before removing the MissionStatement tag, verify that:
- External systems relying on the MissionStatement tag for searching/indexing are updated or can handle this removal
- Tests are updated to reflect the new behavior, or the MissionStatement tag is restored
- Any indexing functionality documented in relevant issues is aligned with this change
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
internal/domain/models/project.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure code formatting and linting pass (make fmt, make lint, make check)
Files:
internal/domain/models/project.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL analysis (go)
- GitHub Check: MegaLinter
🔇 Additional comments (1)
internal/domain/models/project.go (1)
79-82: LGTM! Standardized tag format improves consistency.The change to use the prefixed
project_name:format aligns well with the other tags (project_uid:,project_slug:), improving consistency across the codebase.
Updates the Goa framework version from v3 (latest) to a pinned version v3.22.6 to ensure consistent code generation and build compatibility. This change affects the Makefile and regenerates all Goa-generated code with the specified version. Changes: - Pin GOA_VERSION to v3.22.6 in Makefile - Update goa install command to use the pinned version - Regenerate API code with goa v3.22.6 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Andres Tobon <[email protected]>
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
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (14)
api/project/v1/gen/http/cli/lfx_v2_project_service/cli.gois excluded by!**/gen/**api/project/v1/gen/http/openapi.jsonis excluded by!**/gen/**api/project/v1/gen/http/project_service/client/cli.gois excluded by!**/gen/**api/project/v1/gen/http/project_service/client/client.gois excluded by!**/gen/**api/project/v1/gen/http/project_service/client/encode_decode.gois excluded by!**/gen/**api/project/v1/gen/http/project_service/client/paths.gois excluded by!**/gen/**api/project/v1/gen/http/project_service/client/types.gois excluded by!**/gen/**api/project/v1/gen/http/project_service/server/encode_decode.gois excluded by!**/gen/**api/project/v1/gen/http/project_service/server/paths.gois excluded by!**/gen/**api/project/v1/gen/http/project_service/server/server.gois excluded by!**/gen/**api/project/v1/gen/http/project_service/server/types.gois excluded by!**/gen/**api/project/v1/gen/project_service/client.gois excluded by!**/gen/**api/project/v1/gen/project_service/endpoints.gois excluded by!**/gen/**api/project/v1/gen/project_service/service.gois excluded by!**/gen/**
📒 Files selected for processing (1)
Makefile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: MegaLinter
- GitHub Check: Build and Test
🔇 Additional comments (1)
Makefile (1)
72-72: Installation path correctly aligns with version pinning.The updated installation command properly uses the pinned version from
GOA_VERSIONinstead of@latest, ensuring the exact version specified in line 13 is installed. This follows standard Go module installation syntax.
Updates test expectations in project_test.go to align with the new tag format that only includes prefixed tags (project_uid:, project_slug:, project_name:) and removes unprefixed tags, parent_uid tags, description tags, and mission statement tags. Changes: - Remove expectations for unprefixed UID and Slug tags - Remove expectations for parent_uid tags - Remove expectations for description tags - Remove expectations for mission statement tags - Update Name tag expectations to use project_name: prefix - All tests now pass with the new tag format 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Andres Tobon <[email protected]>
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Summary
ProjectBaseandProjectSettingsmodelsproject_uid:,project_slug:,project_name:)This ensures only necessary and properly formatted tags are sent to the indexer service.
Ticket
https://linuxfoundation.atlassian.net/browse/LFXV2-911
🤖 Generated with Claude Code