Skip to content

MTA-6439 Documentation of Dockerfile Not Generated by generate helm#343

Open
Pkylas007 wants to merge 4 commits intomainfrom
mta-6439-dockerfile-values-generate-helm
Open

MTA-6439 Documentation of Dockerfile Not Generated by generate helm#343
Pkylas007 wants to merge 4 commits intomainfrom
mta-6439-dockerfile-values-generate-helm

Conversation

@Pkylas007
Copy link
Copy Markdown
Collaborator

@Pkylas007 Pkylas007 commented Apr 8, 2026

JIRA

Version

  • 8.1.0

Preview

Summary by CodeRabbit

  • Documentation
    • Added a step‑by‑step procedure for generating deployment assets from a discovery manifest merged with a Helm values file, including required chart layout, example values/Dockerfile templates, command examples, and verification steps.
    • Added an example discovery manifest as a reference and clarified field naming.
    • Improved metadata, formatting, and note structure for clearer, more consistent documentation and explicit precedence rules for configuration sources.

Signed-off-by: Prabha Kylasamiyer Sundara Rajan <pkylasam@pkylasam-thinkpadp16vgen1.bengluru.csb>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 8, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 44397706-de53-49af-81a4-8c0ce067a616

📥 Commits

Reviewing files that changed from the base of the PR and between 862d9ee and 2bc38bc.

📒 Files selected for processing (2)
  • docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc
  • docs/topics/mta-cli/ref_discovery_manifest_sample.adoc
✅ Files skipped from review due to trivial changes (2)
  • docs/topics/mta-cli/ref_discovery_manifest_sample.adoc
  • docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc

📝 Walkthrough

Walkthrough

Added two documentation topics to the generating-assets assembly: a sample discovery manifest reference and a procedure for generating deployment assets using a Helm values file; one existing AsciiDoc module was converted from PROCEDURE to REFERENCE and had formatting adjustments.

Changes

Cohort / File(s) Summary
Assembly update
assemblies/cli-guide/assembly_generating-assets.adoc
Included two new topics into the generating-platform-assets assembly: a discovery manifest reference and a values-file-based deployment-assets procedure.
New procedure
docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc
Added a procedure documenting mta-cli generate helm with --chart-dir, --input, --set, and --output-dir; specifies Helm chart layout (templates/, files/konveyor/, values.yaml), shows examples, defines precedence rules (discovery manifest wins on duplicate keys), and adds verification steps.
Reference example
docs/topics/mta-cli/ref_discovery_manifest_sample.adoc
Changed module type from PROCEDURE to REFERENCE, updated title/abstract to “Example discovery manifest”, reformatted a NOTE into an AsciiDoc [NOTE] block, removed an extra blank line, and adjusted a field name in the note (buildpacksbuildPacks).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested reviewers

  • eemcmullan
  • ibragins

Poem

🐰 I nibble at manifests under moonlight,
Values and templates tucked in just right,
A sample hops in, tidy and neat,
Procedures and references now gently meet,
Hooray — docs ready, my work's a delight! 🥕📘

🚥 Pre-merge checks | ✅ 3
✅ 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 directly references the JIRA ticket MTA-6439 and describes the main change: adding documentation for a known issue where Dockerfile is not generated by the generate helm command.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mta-6439-dockerfile-values-generate-helm

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.

@Pkylas007
Copy link
Copy Markdown
Collaborator Author

@gciavarrini I have made the updates that you mentioned in #312. Could you let me know if they are fine, please?

Copy link
Copy Markdown

@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

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/ref_discovery_manifest_sample.adoc (1)

43-45: ⚠️ Potential issue | 🟡 Minor

Keep buildpack key naming consistent between sample and note.

The sample uses buildPacks, while the note says buildpacks. Use one canonical form across both lines to avoid ambiguity for readers copying the manifest.

Also applies to: 50-50

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/topics/mta-cli/ref_discovery_manifest_sample.adoc` around lines 43 - 45,
The sample and the explanatory note use inconsistent keys: `buildPacks` (in the
sample) vs `buildpacks` (in the note); pick the canonical key `buildpacks` and
update the sample entry (the sequence starting with `- docker://...`) to use
`buildpacks` and also update the note text to match exactly, ensuring both
occurrences (`buildPacks` and `buildpacks`) are normalized to the chosen
canonical form.
🧹 Nitpick comments (1)
docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc (1)

28-31: Use a boolean (not a quoted string) for database.enabled in the example.

enabled: "true" documents this as a string, but the template uses it as a boolean condition. This can mislead users and produce wrong behavior if someone sets "false" (non-empty string still evaluates truthy in Helm templates).

Proposed doc fix
 manifest:
   name: my-app   
   database:
-    enabled: "true"
+    enabled: true
     host: prod-postgres.company.com    

Also applies to: 45-47

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc` around
lines 28 - 31, The example shows database.enabled as a quoted string ("true")
which is misleading because the Helm template treats database.enabled as a
boolean; change the example to use an unquoted boolean (enabled: true) for the
database.enabled key (and any other occurrences such as the later instance at
lines referred to in the comment) so the value is a proper YAML boolean and will
evaluate correctly in templates that rely on database.enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc`:
- Around line 40-41: Replace the incorrect AsciiDoc code block language tag
"source, yaml" with "source,dockerfile" for the Dockerfile examples so syntax
highlighting renders correctly; locate the two occurrences of the literal string
"source, yaml" in
docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc (around the
shown snippet and the second occurrence at lines ~80-81) and update them to
"source,dockerfile".

---

Outside diff comments:
In `@docs/topics/mta-cli/ref_discovery_manifest_sample.adoc`:
- Around line 43-45: The sample and the explanatory note use inconsistent keys:
`buildPacks` (in the sample) vs `buildpacks` (in the note); pick the canonical
key `buildpacks` and update the sample entry (the sequence starting with `-
docker://...`) to use `buildpacks` and also update the note text to match
exactly, ensuring both occurrences (`buildPacks` and `buildpacks`) are
normalized to the chosen canonical form.

---

Nitpick comments:
In `@docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc`:
- Around line 28-31: The example shows database.enabled as a quoted string
("true") which is misleading because the Helm template treats database.enabled
as a boolean; change the example to use an unquoted boolean (enabled: true) for
the database.enabled key (and any other occurrences such as the later instance
at lines referred to in the comment) so the value is a proper YAML boolean and
will evaluate correctly in templates that rely on database.enabled.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 12f8abdf-6d37-475f-b99c-61a95fcf68ea

📥 Commits

Reviewing files that changed from the base of the PR and between cd4dc5a and 0d3acbc.

📒 Files selected for processing (3)
  • assemblies/cli-guide/assembly_generating-assets.adoc
  • docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc
  • docs/topics/mta-cli/ref_discovery_manifest_sample.adoc

Prabha Kylasamiyer Sundara Rajan added 2 commits April 8, 2026 12:00
Signed-off-by: Prabha Kylasamiyer Sundara Rajan <pkylasam@pkylasam-thinkpadp16vgen1.bengluru.csb>
Signed-off-by: Prabha Kylasamiyer Sundara Rajan <pkylasam@pkylasam-thinkpadp16vgen1.bengluru.csb>
Copy link
Copy Markdown

@gciavarrini gciavarrini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Thanks for fixing this :)

Signed-off-by: Prabha Kylasamiyer Sundara Rajan <pkylasam@pkylasam-thinkpadp16vgen1.bengluru.csb>
@Pkylas007 Pkylas007 requested a review from rromannissen April 8, 2026 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants