Skip to content

Conversation

@jortel
Copy link
Contributor

@jortel jortel commented Oct 27, 2025

closes #23

Summary by CodeRabbit

  • New Features
    • Applications are automatically tagged with a standardized platform tag during the fetch process.
    • Platform providers now supply a platform name used for tagging, ensuring consistent tag categorization across applications.
    • Tagging is applied early in the fetch flow and surfaced in activity/log messages on replacement errors.

Signed-off-by: Jeff Ortel <[email protected]>
@coderabbitai
Copy link

coderabbitai bot commented Oct 27, 2025

Walkthrough

Adds platform tagging: introduces TagSource and TagCategory constants and Fetch.setTag(p Provider); calls setTag in Fetch.Run. Adds Tag() string to the Provider interface and implements it for the Cloud Foundry provider returning "Cloud Foundry".

Changes

Cohort / File(s) Summary
Fetch command updates
cmd/fetch.go
Added public constants TagSource and TagCategory; implemented func (a *Fetch) setTag(p Provider) error which ensures the Platform tag category and a provider tag exist and replaces the application's tags; integrated setTag call into Fetch.Run after provider selection with error propagation.
Provider interface change
cmd/action.go
Extended Provider interface with new method signature Tag() string.
Cloud Foundry provider
cmd/cloudfoundry/provider.go
Implemented func (p *Provider) Tag() string returning "Cloud Foundry" to satisfy the new interface method.

Sequence Diagram(s)

sequenceDiagram
    participant Run as Fetch.Run
    participant Provider as Provider Selection
    participant setTag as Fetch.setTag
    participant API as Tag API / App
    participant FetchM as Manifest Fetch

    Run->>Provider: select provider
    Provider-->>Run: provider (implements Tag())

    rect rgba(200,220,240,0.25)
      Note over Run,setTag: New tagging flow
      Run->>setTag: setTag(provider)
      setTag->>API: ensure TagCategory (TagCategory, TagSource)
      API-->>setTag: category OK
      setTag->>API: ensure Tag (provider.Tag())
      API-->>setTag: tag OK
      setTag->>API: Replace application's tags with new tag
      API-->>setTag: replace result / error
      setTag-->>Run: return error? / ok
    end

    Run->>FetchM: fetch manifest
    FetchM-->>Run: manifest data
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Verify interface change impact across codebase (implementations must implement Tag()).
  • Inspect setTag logic for correct creation/ensuring of TagCategory and Tag and correct use of Replace on application tags.
  • Confirm error handling in Fetch.Run prevents unwanted side effects and logs appropriately.

Poem

🐰 I hop and tag with gentle cheer,
Platform named, the path is clear,
"Cloud Foundry" stamped with source in hand,
I replace old tags across the land,
Then fetch the manifest — hoppity-scan!

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Linked Issues Check ⚠️ Warning Issue #23 specifies the tag should have key platform (lowercase) with value Platform.Kind and source platform-discovery. The code implements the source correctly as TagSource = "platform-discovery" and the value correctly via provider.Tag() returning "Cloud Foundry". However, the code uses TagCategory = "Platform" (capitalized) rather than the lowercase platform specified in the requirement. This appears to be a mismatch in the tag key naming that does not fully comply with the stated requirement. Update TagCategory from "Platform" to "platform" (lowercase) to match the requirement in issue #23 that specifies the tag key should be platform. This ensures the tag structure precisely matches the specification of "key platform and value Platform.Kind" from the linked issue.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Tag application with platform=Cloud Foundry" directly describes the main objective of the pull request: introducing platform tagging functionality and implementing it for the Cloud Foundry provider. The title is specific, concise, and clearly conveys the primary change being made to tag applications with platform information. While the changes also introduce the tagging infrastructure more broadly (through the Provider interface), the title appropriately focuses on the concrete outcome from the user's perspective.
Out of Scope Changes Check ✅ Passed All changes in the pull request are directly aligned with the requirements of issue #23. The changes to cmd/fetch.go add the tagging infrastructure and integrate it into the fetch flow, the changes to cmd/action.go extend the Provider interface to support tagging, and the changes to cmd/cloudfoundry/provider.go implement the Tag() method for the Cloud Foundry provider. No out-of-scope refactoring, cleanup, or unrelated functionality has been introduced.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3689e4a and 05ba3da.

📒 Files selected for processing (1)
  • cmd/fetch.go (3 hunks)
🔇 Additional comments (1)
cmd/fetch.go (1)

34-37: Correct placement of tagging in the flow.

The setTag() invocation is correctly positioned after provider selection and before manifest fetch, which aligns with the requirement to tag when the manifest is fetched.

Signed-off-by: Jeff Ortel <[email protected]>
Copy link

@coderabbitai coderabbitai bot left a 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: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 05ba3da and 1bd5abd.

📒 Files selected for processing (3)
  • cmd/action.go (1 hunks)
  • cmd/cloudfoundry/provider.go (1 hunks)
  • cmd/fetch.go (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
cmd/fetch.go (1)
cmd/action.go (1)
  • Provider (121-126)
cmd/cloudfoundry/provider.go (1)
cmd/action.go (1)
  • Provider (121-126)
🔇 Additional comments (4)
cmd/cloudfoundry/provider.go (1)

102-105: LGTM!

The Tag() method implementation is clean and correctly implements the Provider interface requirement.

cmd/action.go (1)

125-125: LGTM!

The interface extension is clean and follows good design practices by allowing each provider to define its own platform tag.

cmd/fetch.go (2)

5-8: LGTM!

The constants are well-defined and align with the PR objectives. The TagSource value "platform-discovery" matches the requirement to use platform-discovery as the tag's source.


33-36: LGTM!

The setTag invocation is correctly placed in the execution flow and properly handles errors.

@jortel jortel added the cherry-pick/release-0.8 This PR should be cherry-picked to release-0.8 branch label Oct 28, 2025
@jortel jortel merged commit 1ac1586 into konveyor:main Oct 30, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherry-pick/release-0.8 This PR should be cherry-picked to release-0.8 branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Manage platform tags.

2 participants