Skip to content

MTA-6439: grammar issues#352

Merged
anarnold97 merged 5 commits intomigtools:mainfrom
anarnold97:grammar-errors-MTA-6439
Apr 14, 2026
Merged

MTA-6439: grammar issues#352
anarnold97 merged 5 commits intomigtools:mainfrom
anarnold97:grammar-errors-MTA-6439

Conversation

@anarnold97
Copy link
Copy Markdown
Collaborator

@anarnold97 anarnold97 commented Apr 13, 2026

Summary by CodeRabbit

  • Documentation
    • Enhanced deployment asset generation procedure with clearer prerequisite requirements, expanded command parameter explanations, and clarified configuration precedence rules for Helm chart customization.

PREVIEW

coderabbitai[bot]

This comment was marked as outdated.

Comment thread docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc Outdated
Comment thread docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc Outdated
@anarnold97 anarnold97 force-pushed the grammar-errors-MTA-6439 branch from eab5940 to 86d46da Compare April 13, 2026 18:51
@migtools migtools deleted a comment from coderabbitai Bot Apr 13, 2026
Copy link
Copy Markdown
Collaborator

@Pkylas007 Pkylas007 left a comment

Choose a reason for hiding this comment

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

@anarnold97 Thanks for re-framing the sentences! Could you please add back details about the locations where the template files are stored? It was requested by Ramon. Thank you!

"To use the helm templates to generate assets, save the Kubernetes YAML templates, for example, ConfigMap or Deployment, in the templates directory present in the root directory of the Helm chart. You must save non-Kubernetes templates, for example, Dockerfile, in the files/konveyor path in the Helm chart root directory and save the values.yaml file at the root directory of the Helm chart."

Comment thread docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc Outdated
Comment thread docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc Outdated
Comment thread docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc Outdated
Comment thread docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc Outdated
Comment thread docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc Outdated
Copy link
Copy Markdown
Collaborator

@mpershina mpershina left a comment

Choose a reason for hiding this comment

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

Added some suggestions and fixes, otherwise, LGTM

@mpershina
Copy link
Copy Markdown
Collaborator

mpershina commented Apr 14, 2026

@anarnold97 Thanks for re-framing the sentences! Could you please add back details about the locations where the template files are stored? It was requested by Ramon. Thank you!

"To use the helm templates to generate assets, save the Kubernetes YAML templates, for example, ConfigMap or Deployment, in the templates directory present in the root directory of the Helm chart. You must save non-Kubernetes templates, for example, Dockerfile, in the files/konveyor path in the Helm chart root directory and save the values.yaml file at the root directory of the Helm chart."

@Pkylas007 This one actually looks like it should be a part of a procedure or even prerequisites. We do not instruct users to do something outside of a procedure/prerequisites, apart from requesting actions in some admonition, for example, Important. Also, this text could use some simplification

Agreed! I wouldn't mind adding this instruction as an IMPORTANT admonition. This module is a result of an actual internal stakeholder request for a few things to be documented, including where each file is located :)

Comment thread docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc Outdated
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 14, 2026

Warning

Rate limit exceeded

@anarnold97 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 28 minutes and 1 seconds before requesting another review.

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 28 minutes and 1 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 795c018a-de84-4a58-9003-cebeff6f9e01

📥 Commits

Reviewing files that changed from the base of the PR and between 0d099a8 and 58aab80.

📒 Files selected for processing (1)
  • docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc
📝 Walkthrough

Walkthrough

Updated AsciiDoc documentation for generating deployment assets with Helm charts. Changes clarify prerequisites for file placement, restructure examples with improved formatting, add explicit placeholder explanations, and reorganize priority ordering for configuration values (--set, discovery manifest, then values.yaml).

Changes

Cohort / File(s) Summary
Documentation Update
docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc
Restructured procedure describing Helm chart deployment manifest generation: revised prerequisites with file placement requirements, expanded command explanation with "where" section for placeholders, replaced NOTE-based priority text with structured list, updated code block substitution attributes to [subs="+quotes"], and adjusted verification wording.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • Pkylas007
  • gciavarrini
  • rromannissen

Poem

🐰✨ A docstring hops with clarity so bright,
Configuration flows, ordered just right—
Prerequisites placed, like carrots in rows,
Where Helm and values dance—on it goes!
The procedure now whispers, no mysteries at night! 📜

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'MTA-6439: grammar issues' is vague and does not accurately reflect the main changes, which include substantial procedural restructuring, example updates, and critical location/precedence documentation—not primarily grammar fixes. Revise the title to reflect the primary change, such as 'Document Helm chart directory structure and deployment manifest generation precedence' or 'Clarify deployment manifest generation from discovery manifest and values.yaml'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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

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.

Comment thread docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc Outdated
Comment thread docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc Outdated
Comment thread docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc Outdated
Comment thread docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc Outdated
Signed-off-by: A.Arnold <anarnold@redhat.com>
@anarnold97 anarnold97 force-pushed the grammar-errors-MTA-6439 branch from 2bdc19b to 148a4d7 Compare April 14, 2026 12:23
Comment thread docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc Outdated
…e.adoc

Co-authored-by: Mariya Pershina <53339200+mpershina@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@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

🤖 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 59-68: The placeholders for the discovery manifest are
inconsistent: the command uses `<path_to_discovery-manifest>` (hyphen) while the
where section defines `<path_to_discovery_manifest>` (underscore); update all
occurrences so they match exactly (choose one form and apply it to the command
example and the where definitions) — specifically edit the command line that
contains `--input _<path_to_discovery-manifest>_` and/or the where entry
`_<path_to_discovery_manifest>_` so both use the same token (e.g.,
`<path_to_discovery_manifest>`), ensuring `--input`, the inline code sample, and
the corresponding where description all reference the identical placeholder.
🪄 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: 61fe2ab3-e266-4c69-b72d-ced781f776c7

📥 Commits

Reviewing files that changed from the base of the PR and between 6f61e76 and 0d099a8.

📒 Files selected for processing (1)
  • docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc

Comment thread docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc Outdated
Comment thread docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc Outdated
anarnold97 and others added 2 commits April 14, 2026 14:00
…e.adoc

Co-authored-by: Mariya Pershina <53339200+mpershina@users.noreply.github.com>
…e.adoc

Co-authored-by: Mariya Pershina <53339200+mpershina@users.noreply.github.com>
Copy link
Copy Markdown
Collaborator

@mpershina mpershina left a comment

Choose a reason for hiding this comment

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

LGTM now!

Comment on lines +65 to +68
where:
`_<path_to_helm_chart>_`:: Specifies the directory containing the Helm chart.
`_<path_to_discovery_manifest>_`:: Specifies the location of the discovery manifest file.
`_<location_of_deployment_manifests>_`:: Specifies the output directory for the generated manifests.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@mpershina -this look weird when it renders, please can you have a look-see:

image

Is it ok to change to

image

Thanks

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Oh, I think I know where the issue might be :-D
That's why we always need a preview. So, we basically need to have an empty line after where. I added my suggestion above

Comment thread docs/topics/mta-cli/proc_generate_deployment_assets_values_file.adoc Outdated
…e.adoc

Co-authored-by: Mariya Pershina <53339200+mpershina@users.noreply.github.com>
+
The variable priority hierarchy for generating deployment manifests is as follows:

* **Priority 1: The `--set` command flag**: Any value specified with this flag has the highest priority and overrides variables defined in either the `values.yaml` file or the discovery manifest.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* **Priority 1: The `--set` command flag**: Any value specified with this flag has the highest priority and overrides variables defined in either the `values.yaml` file or the discovery manifest.
* **(Priority 1) The `--set` command flag**: Any value specified with this flag has the highest priority and overrides variables defined in either the `values.yaml` file or the discovery manifest.

Please shall we remove too many colons in a line here? The format looks a little weird, WDYT?

@anarnold97 anarnold97 merged commit 7e49437 into migtools:main Apr 14, 2026
2 checks passed
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.

3 participants