[MTA 8.1.1] Updating MTA documentation for asset generation without discovery manifests#360
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 44 minutes and 20 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughDocumentation updates clarifying the discovery-manifest-driven generation workflow, explaining how the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/topics/mta-cli/proc_generating-deployment-manifest.adoc (1)
41-41:⚠️ Potential issue | 🟡 MinorUse underscore-style placeholder token in the command example.
_<path_to_discovery-manifest>_should use underscores between words to match repository placeholder conventions.Suggested fix
---input _<path_to_discovery-manifest>_ \ +--input _<path_to_discovery_manifest>_ \Based on learnings: In this documentation repo’s AsciiDoc files, placeholder values in command examples must separate words with underscores, not hyphens.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/topics/mta-cli/proc_generating-deployment-manifest.adoc` at line 41, The placeholder token `_ <path_to_discovery-manifest>_` uses a hyphen but must follow the repo convention of underscore-separated words; update the command example to use `_<path_to_discovery_manifest>_` (replace the hyphen with an underscore) so the placeholder in the docs matches other AsciiDoc examples.
🧹 Nitpick comments (1)
assemblies/cli-guide/assembly_generating-assets.adoc (1)
34-34: Remove commented-out legacy sentence to keep docs clean.This commented block is no longer rendered and increases maintenance noise; better to remove it from the source.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assemblies/cli-guide/assembly_generating-assets.adoc` at line 34, Remove the commented-out legacy sentence "//by using the discovery manifest. The deployment manifest is generated by using a templating engine, such as Helm, that converts the discovery manifest into a Kubernetes-native format. You can also use this command to generate non-Kubernetes manifests, such as a Dockerfile or a configuration file." from assembly_generating-assets.adoc to eliminate dead/commented content; simply delete that entire commented line so only active, rendered documentation remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docs/topics/mta-cli/proc_generating-deployment-manifest.adoc`:
- Line 41: The placeholder token `_ <path_to_discovery-manifest>_` uses a hyphen
but must follow the repo convention of underscore-separated words; update the
command example to use `_<path_to_discovery_manifest>_` (replace the hyphen with
an underscore) so the placeholder in the docs matches other AsciiDoc examples.
---
Nitpick comments:
In `@assemblies/cli-guide/assembly_generating-assets.adoc`:
- Line 34: Remove the commented-out legacy sentence "//by using the discovery
manifest. The deployment manifest is generated by using a templating engine,
such as Helm, that converts the discovery manifest into a Kubernetes-native
format. You can also use this command to generate non-Kubernetes manifests, such
as a Dockerfile or a configuration file." from assembly_generating-assets.adoc
to eliminate dead/commented content; simply delete that entire commented line so
only active, rendered documentation remains.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 01433388-8a2a-480b-bc62-dd3455204bc5
📒 Files selected for processing (3)
assemblies/cli-guide/assembly_generating-assets.adocdocs/topics/mta-cli/proc_generating-deployment-manifest.adocdocs/topics/mta-cli/ref_discover-generate-command-options.adoc
anarnold97
left a comment
There was a problem hiding this comment.
This PR looks largely correct in its approach to "Asset Generation without discovery manifests." It shifts from a mandatory discovery-led workflow to a more flexible code-led approach.
Please can you check the "Transform" section of the CLI guide in the PR.
Does MTA-6788 introduce specific new flags to "simulate" a manifest? then we might have to list those in the CLI parameter tables.
Hi @sjd78, could you please confirm whether there are some flags that users need to use to simulate a discovery manifest for deployment manifest generation by using the CLI? Or is it only about replacing the |
I'm not sure about the impact on the cli for the generators. Since generators can be anything, they may not consider the discovery manifest at all. AFAIK, the |
@dymurray @rromannissen based on what Scott said, do we even want to mention anything about asset generation without a discovery manifest in the CLI doc? |
|
@mpershina I think we just remove it; not necessary to call out. But if we do this looks fine as is |
anarnold97
left a comment
There was a problem hiding this comment.
LGTM - nothing to stop merging. Any suggestions are merely very minor stylistic suggestions.
Co-authored-by: Andy Arnold <anarnold@redhat.com>
Tracked under:
Previews:
Summary by CodeRabbit
generatecommand supports both Kubernetes and non-Kubernetes manifests.