MTA-6826, 6827, 6828, 6829, 6830, 6831 CQA April work for rules development guide#359
MTA-6826, 6827, 6828, 6829, 6830, 6831 CQA April work for rules development guide#359Pkylas007 wants to merge 4 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDocumentation-only edits across the Rules Development guide: replaced hardcoded product/CLI names with parameterized placeholders, reordered Asciidoc conditionals, renamed/retitled provider labels and inline code formatting, removed one rule-conditions include, and updated a cross-reference target. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/topics/rules-development/yaml-java-locations.adoc (1)
114-123:⚠️ Potential issue | 🟠 MajorAlign location keyword with example (
FIELDvsFIELD_DECLARATION).Line 114 lists
FIELD, but the example at Lines 121-123 usesFIELD_DECLARATION. Please make these consistent; otherwise readers may use an invalid location value in rules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/topics/rules-development/yaml-java-locations.adoc` around lines 114 - 123, The location keyword is inconsistent: the table lists `FIELD` but the example uses `FIELD_DECLARATION`; update one so both match. Edit the example in the java.referenced block or the table entry so the location value is the same (choose either `FIELD` or `FIELD_DECLARATION`) and ensure the example's when -> java.referenced -> location uses that exact token; keep the rest (pattern: javax.jms.QueueConnectionFactory) unchanged.
🤖 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/rules-development/yaml-dotnet-provider.adoc`:
- Line 12: Replace the standalone acronym "gRPC" in the sentence that begins
"The `C#` provider uses a gRPC interface..." by expanding it on first use as
"Google Remote Procedure Call (gRPC)" and then retain the abbreviation "gRPC"
for subsequent mentions; update the sentence containing the term `gRPC` so it
reads with the full name followed by the abbreviation.
In `@docs/topics/rules-development/yaml-java-locations.adoc`:
- Line 146: The DESCRIPTION for the `CLASS` declaration is wrong: update the row
that reads "`CLASS` (declaration) Matches against a given method declaration" to
describe class declarations instead—e.g., say "`CLASS` (declaration) Matches
against a given class declaration; can be coupled with an annotation match (see
Annotation inspection)". Modify the text for the `CLASS` entry in
yaml-java-locations.adoc to replace "method declaration" with "class
declaration" and keep the note about coupling with annotations/reference to the
Annotation inspection page.
In `@docs/topics/rules-development/yaml-provider-conditions.adoc`:
- Around line 325-329: The row describing the `builtin.json` capability uses the
phrase "an XPath query" which conflicts with earlier terminology (`jsonpath`);
update the phrase "an XPath query" to "a JSONPath query" (or "a jsonpath
expression") so the `builtin.json` row consistently references JSONPath/jsonpath
like the earlier JSON capability text.
---
Outside diff comments:
In `@docs/topics/rules-development/yaml-java-locations.adoc`:
- Around line 114-123: The location keyword is inconsistent: the table lists
`FIELD` but the example uses `FIELD_DECLARATION`; update one so both match. Edit
the example in the java.referenced block or the table entry so the location
value is the same (choose either `FIELD` or `FIELD_DECLARATION`) and ensure the
example's when -> java.referenced -> location uses that exact token; keep the
rest (pattern: javax.jms.QueueConnectionFactory) unchanged.
🪄 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: 67d7ee91-99a7-4c2f-a69a-16d41e10c933
📒 Files selected for processing (7)
assemblies/rules-development-guide/assembly_creating-rule.adocassemblies/rules-development-guide/assembly_java-conditions-detailed.adocassemblies/rules-development-guide/assembly_rules-introduction.adocdocs/topics/rules-development/yaml-dotnet-provider.adocdocs/topics/rules-development/yaml-java-locations.adocdocs/topics/rules-development/yaml-provider-conditions.adocdocs/topics/rules-development/yaml-rule-conditions.adoc
💤 Files with no reviewable changes (1)
- docs/topics/rules-development/yaml-rule-conditions.adoc
Pkylas007
changed the title
Pkylas007
changed the title
JIRA CQA Analysis and Remediation Summary: PR #359Document Title: Configuring and using rules for an MTA analysis Analysis Date: 2026-04-15 Genuine Issues Identified: 10 Pull Request #359 successfully addresses the genuine issues identified in the April 2026 Content Quality Assurance (CQA) report by remediating findings across six JIRA issues (MTA-6826 to MTA-6831). 1. Critical Structural Repairs (MTA-6826)This JIRA issue focuses on structural integrity and orphaned files.
2. Branding and Attribute Compliance (MTA-6827)This JIRA issue ensures product naming meets branding standards.
3. Technical Formatting and Accuracy (MTA-6828)This JIRA issue standardizes YAML properties and provider names.
4. Readability and Grammar Remediation (MTA-6829)This JIRA issue targets dense prose and passive construction.
5. Clarity and Style Standardization (MTA-6830 & MTA-6831)These JIRA issues cover acronym definitions and overall style polish.
|
anarnold97
left a comment
anarnold97
left a comment
There was a problem hiding this comment.
DITA report
master.adoc
-
12:1 warning Assign [role="_abstract"] AsciiDocDITA.ShortDescription
to a paragraph to use it as
in DITA. -
12:1 warning Content other than additional AsciiDocDITA.AssemblyContents
resources cannot follow
include directives. -
39:1 warning Content other than links AsciiDocDITA.RelatedLinks
cannot be mapped to DITA
related-links.Are these all false positives?
Hey Andy,
|
vashirova
left a comment
vashirova
left a comment
There was a problem hiding this comment.
@Pkylas007 peer review completed, see my comments. This is a great start! I've noticed some outstanding issues that are applicable to several files:
- Abstracts - some of them exceed character limit of 300, end with a colon (:), and do not work as standalone text.
- Leftover "refer to" instances.
- Complex ifdef syntax in assemblies.
- Leftover inconsistencies in "rule sets" spelling.
- Leftover passive voice.
- Broken syntax for italics.
- Leftover incorrectly formatted user-replaced values (must be <user_replaced>).
- Not everything from the linked ticket was addressed.
Let me know when you'd need a second round of review. Thank you.
There was a problem hiding this comment.
Outside of scope suggestions for this and other files but must before migration to AEM:
- Rewrite abstract to clarify wording 'capability in the when condition' (when in backticks perhaps?) plus colon (:) is not supported for short description in AEM. See https://redhat-documentation.github.io/supplementary-style-guide/#_core_principles_for_writing_a_helpful_short_description.
- Replace 'refer to' (for example, with 'see'). See https://www.ibm.com/docs/en/ibm-style?topic=word-usage#r.
There was a problem hiding this comment.
Outside of scope suggestions but a must before migration to AEM:
- Shorten/add a new paragraph for abstract as the current one is a bit too long (394 characters instead of 300).
- Also, it is not standalone text, "Additionally..." right afterwards suggests this. I'd recommend to rework this - either to rewrite the current abstract and following text or add a completely new and separate 300 characters short description. See https://redhat-documentation.github.io/supplementary-style-guide/#shortdesc.
|
|
||
| The rules can be used by {mta-dl-plugin} to generate AI-assisted code resolutions. The issue description and metadata in rules are used by the {mta-dl-plugin} to form well-defined contexts. These contexts generate AI-assisted suggestions to resolve issues in the code that are identified through an analysis. | ||
|
|
||
| The rules can be used by {mta-dl-plugin} to generate AI-assisted code resolutions. {mta-dl-plugin} uses the issue description and metadata in rules to form well-defined contexts. These contexts generate AI-assisted suggestions to resolve issues in the code that {ProductShortName} identified through an analysis. |
There was a problem hiding this comment.
How about this rewrite for the first sentence to remove passive voice?
Plus there are more instances of passive voice in this files and others, too. Is it intentional that we are fixing only this specific instance and not everything?
| The rules can be used by {mta-dl-plugin} to generate AI-assisted code resolutions. {mta-dl-plugin} uses the issue description and metadata in rules to form well-defined contexts. These contexts generate AI-assisted suggestions to resolve issues in the code that {ProductShortName} identified through an analysis. | |
| {mta-dl-plugin} can use the rules to generate AI-assisted code resolutions. It uses the issue description and metadata in rules to form well-defined contexts. These contexts generate AI-assisted suggestions to resolve issues in the code that {ProductShortName} identified through an analysis. |
|
|
||
| :_mod-docs-content-type: ASSEMBLY | ||
|
|
||
| ifdef::context[:parent-context-of-java-conditions-capabilities: {context}] |
There was a problem hiding this comment.
I'm not entirely sure what we are trying to achieve with this change, could you elaborate on it please
Furthermore, the ifdef syntax with 2 statements seems complex. Typically we'd use ifdef if an assembly is re-used, however I don't think any of the assemblies in this PR are re-used right now? Could we review all this ifdef syntax for all of assemblies and asses if it could be simplified or removed? This could help with the unclosed parent context issue.
There was a problem hiding this comment.
Thank you for your comment. I'm sorry but the CQA and DITA runs did not catch any unclosed parent context issue. I think the unset issue that is flagged was a false positive.
There was a problem hiding this comment.
I like that this short description includes the who and the what. The only missing piece according to our guidance https://redhat-documentation.github.io/supplementary-style-guide/#shortdesc is missing user-focused language ("This guide is intended " is self-referential and must be avoided). I suggest to reword it for DITA.
|
|
||
| * You installed the `ilspycmd` and `paket` dependencies. | ||
| * You installed the `dotnet tools` and exported the `dotnet tools` path by using the `export PATH="$PATH:<path/to/.dotnet/tools"` command. | ||
| * You installed the `dotnet tools` and exported the `dotnet tools` path by using the `export PATH="$PATH:_<path_to_.dotnet_tools>_"` command. |
There was a problem hiding this comment.
The newly added underscores do not make this user-replaceable value in italic, I'm not sure why. Perhaps we can try double underscores to read it as one piece?
| * You installed the `dotnet tools` and exported the `dotnet tools` path by using the `export PATH="$PATH:_<path_to_.dotnet_tools>_"` command. | |
| * You installed the `dotnet tools` and exported the `dotnet tools` path by using the `export PATH="$PATH:__<path_to_.dotnet_tools>__"` command. |
There was a problem hiding this comment.
I agree, the formatting requirements seem complicated and make rendering very tricky.
| @@ -62,7 +62,7 @@ You can use a custom rule to check if {ProductShortName} triggers an incident wh | |||
|
|
|||
| [source, terminal] | |||
There was a problem hiding this comment.
| [source, terminal] | |
| [source, terminal,subs="+quotes"] |
Changes on line 65 do not work, the user-replaced value is not in italics.
|
|
||
| [role="_abstract"] | ||
| You can refer to example rules that describe how to create a YAML rule. This assumes that you already have MTA installed. See the MTA CLI Guide for installation instructions. | ||
| You can refer to example rules that describe how to create a YAML rule. This assumes that you already have {ProductShortName} installed. See the {ProductShortName} {CLIName} Guide for installation instructions. |
There was a problem hiding this comment.
I believe this abstract can be improved. First of all, it again uses "refer to" which isn't recommended for docs in general, see https://www.ibm.com/docs/en/ibm-style?topic=word-usage#r. Second, we can be more direct and start with an imperative. Next, as a general rule, I'd argue that referring users to another piece of documentation doesn't belong in short descriptions. Short descriptions meant to help user navigate and should work as a standalone piece. I strongly suggest to review this and other short descriptions.
| |Location |Description |Rule condition example | ||
|
|
||
| |TYPE|Matches against all types, including classes, interfaces, enums, and annotation types that appear anywhere. | ||
| |`TYPE`|Matches against all types, including classes, interfaces, enums, and annotation types that appear anywhere. |
There was a problem hiding this comment.
| |`TYPE`|Matches against all types, including classes, interfaces, enums, and annotation types that appear anywhere. | |
| |`TYPE`|Matches against all types, including classes, interfaces, `enums`, and annotation types that appear anywhere. |
| mkdir /home/<USER>/data/ | ||
| touch /home/<USER>/data/jboss-web.xml | ||
| touch /home/<USER>/data/pom.xml |
There was a problem hiding this comment.
This happens to catch my eye. These commands should be separated (one command per one command block) and shell prompt (I think $, based on the previous step) should be added to each.
There was a problem hiding this comment.
Thank you for your suggestion! This topic requires a re-write which is not strictly within the scope of work tracked in this PR. I'll get back to you on this 👍
|
Also can we be careful when running DITA output: I think as the Also, I know Thanks |
Thanks for the comment. Previously, the team agreed that The |
Pkylas007
changed the title
I disagree with the last point. I made sure that all points in the JIRA were covered. Thank you. |
Signed-off-by: Prabha Kylasamiyer Sundara Rajan <pkylasam@pkylasam-thinkpadp16vgen1.bengluru.csb>
Signed-off-by: Prabha Kylasamiyer Sundara Rajan <pkylasam@pkylasam-thinkpadp16vgen1.bengluru.csb>
Signed-off-by: Prabha Kylasamiyer Sundara Rajan <pkylasam@pkylasam-thinkpadp16vgen1.bengluru.csb>
|
Hi @vashirova, I think I've addressed the DITA validation and vale validation issues. Can you please review the updates? I've also run the DITA and vale validation to ensure that I've addressed as many issues as I can. Thanks! |
Pkylas007
changed the title
Pkylas007
changed the title
Signed-off-by: Prabha Kylasamiyer Sundara Rajan <pkylasam@pkylasam-thinkpadp16vgen1.bengluru.csb>
vashirova
left a comment
vashirova
left a comment
There was a problem hiding this comment.
@Pkylas007 thank you for your patience! Peer review completed, I've added my suggestions.
|
|
||
| [role="_abstract"] | ||
| This guide is intended for software engineers who want to create custom YAML-based rules for {ProductName} ({ProductShortName}) tools. | ||
| As an Architect, see the following sections to understand rules and rule syntax so that, you can create custom YAML-based rules for {ProductName} ({ProductShortName}) tools. |
There was a problem hiding this comment.
I believe we can further refine this short description to set expectations for users. How about something like this?
| As an Architect, see the following sections to understand rules and rule syntax so that, you can create custom YAML-based rules for {ProductName} ({ProductShortName}) tools. | |
| You can analyze specific aspects of an application by using custom YAML-based rules for {ProductName} ({ProductShortName}). Creating your own rules helps you identify the use of custom libraries or components that standard migration rules might miss. |
| * Evaluate if a resolution is mandatory through rule category before migrating the applications to the target technologies or platforms. | ||
| * Define labels used by {ProductShortName} to filter rules for an analysis. | ||
|
|
||
| As an Architect, you include information about effort in the rule metadata. Effort is a numerical value that accounts for the complexity in resolving the issue described in the rule. In the metadata, you can add category and label that guides Migrators when they resolve issues in the code. |
There was a problem hiding this comment.
| As an Architect, you include information about effort in the rule metadata. Effort is a numerical value that accounts for the complexity in resolving the issue described in the rule. In the metadata, you can add category and label that guides Migrators when they resolve issues in the code. | |
| As an Architect, you include information about effort in the rule metadata. Effort is a numerical value that accounts for the complexity in resolving the issue described in the rule. In the metadata, you can add a category and a label that guide Migrators when they resolve issues in the code. |
I believe it should be "that guide" since we are talking about category and label?
| include::topics/making-open-source-more-inclusive.adoc[] | ||
|
|
||
| include::assemblies/rules-development-guide/assembly_rules-introduction.adoc[leveloffset=+1] | ||
|
|
There was a problem hiding this comment.
I was under assumption blank lines between includes are a must?
|
|
||
| [role="_abstract"] | ||
| The {ProductFullName} contains the analyzer that uses pluggable providers and rules to analyze static code, dependencies, and other files in your application. | ||
| {ProductFullName} analyzes the application source code to find issues based on user-generated rule and default rule definitions. You can fix issues in an analysis report before migrating the code to a target platform. You can view the rule syntax information to create a new rule or a ruleset. |
There was a problem hiding this comment.
| {ProductFullName} analyzes the application source code to find issues based on user-generated rule and default rule definitions. You can fix issues in an analysis report before migrating the code to a target platform. You can view the rule syntax information to create a new rule or a ruleset. | |
| {ProductFullName} analyzes the application source code to find issues based on user-generated and default rule definitions. You can fix issues in an analysis report before migrating the code to a target platform. You can view the rule syntax information to create a new rule or a ruleset. |
'user-generated' and 'default' modify the same plural noun.
|
|
||
| .Prerequisites | ||
|
|
||
| * You installed {ProductShortName}. See Installing and running the CLI. |
There was a problem hiding this comment.
We could add link here. They are acceptable in procedures.
| = OR condition | ||
|
|
||
| [role="_abstract"] | ||
| The `or` condition performs a logical OR operation on the results of an array of conditions. The condition matches when _any_ of its child conditions match, for example: |
There was a problem hiding this comment.
DITA doesn't support abstracts that end with a colon. How about removing the last part?
| The `or` condition performs a logical OR operation on the results of an array of conditions. The condition matches when _any_ of its child conditions match, for example: | |
| The `or` condition performs a logical OR operation on the results of an array of conditions. The condition matches when _any_ of its child conditions match. |
|
|
||
| [role="_abstract"] | ||
| Hyperlinks can be provided along with a `message` or `tag` action to provide relevant information about the issue that has been discovered: | ||
| As a rule author, you can configure external hyperlinks along with a `message` or `tag` action. Hyperlinks provide relevant documentation that helps you to resolve the issue that {ProductShortName} discovered: |
There was a problem hiding this comment.
DITA doesn't support lists in abstracts so it cannot end with a colon.
| As a rule author, you can configure external hyperlinks along with a `message` or `tag` action. Hyperlinks provide relevant documentation that helps you to resolve the issue that {ProductShortName} discovered: | |
| As a rule author, you can configure external hyperlinks along with a `message` or `tag` action. Hyperlinks provide relevant documentation that helps you to resolve the issue that {ProductShortName} discovered. |
|
|
||
| [role="_abstract"] | ||
| Rule metadata contains a unique rule ID, labels, effort, and category. | ||
| In rule metadata, define a unique rule ID, labels, effort, and category so that the analyzer can select the rules for an analysis and compute the total effort to resolve the issues detected in the source code. |
There was a problem hiding this comment.
While it is not wrong, it reads more like a procedure (define) which doesn't fit the module type (concept). I'd suggest rephrasing it to make it clear this is a concept and not instructional.
|
|
||
| [role="_abstract"] | ||
| You can create a ruleset by placing one or more custom rules in a directory and creating a `ruleset.yaml` file at the directory root. When you perform an analysis, you pass this directory as input to the {ProductShortName} {CLIName} by using the `--rules` option. All rules in this directory are treated as a part of the ruleset defined by the `ruleset.yaml` file. | ||
| You can create a ruleset by placing one or more custom rules in a directory and creating a `ruleset.yaml` file at the directory root. When you perform an analysis, you pass this directory as input to the {ProductShortName} {CLIName} by using the `--rules` option. {ProductShortName} treats all rules in this directory as a part of the ruleset defined by the `ruleset.yaml` file. |
There was a problem hiding this comment.
This appears to be too long, I counted 336 characters. How about this instead?
| You can create a ruleset by placing one or more custom rules in a directory and creating a `ruleset.yaml` file at the directory root. When you perform an analysis, you pass this directory as input to the {ProductShortName} {CLIName} by using the `--rules` option. {ProductShortName} treats all rules in this directory as a part of the ruleset defined by the `ruleset.yaml` file. | |
| You can create a custom ruleset to organize multiple custom rules into a single directory. Defining your rulesets with a `ruleset.yaml` file helps you to efficiently apply them all at once when running an analysis with the {ProductShortName} {CLIName}. |
| :ProductShortName: MTA | ||
| :ProductShortNameLower: mta | ||
| :ProductFullName: migration toolkit for applications (MTA) | ||
| :ProductFullName: Migration toolkit for applications (MTA) |
There was a problem hiding this comment.
I think this should remain as 'migration toolkit for applications' for cases where this attribute is not first in a sentence? Otherwise all instances of {ProductFullName} will resolve to 'Migration toolkit for applications' regardless of their position in the sentence.
| :ProductFullName: Migration toolkit for applications (MTA) | |
| :ProductFullName: migration toolkit for applications (MTA) |
3 participants
JIRA
Version
Preview
Summary by CodeRabbit