DOCPLAN-400: Convert ifeval to ifdef for DITA compatibility - #952
DOCPLAN-400: Convert ifeval to ifdef for DITA compatibility#952danielclowers wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces new ChangesUpstream/downstream attribute-based conditionals
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@documentation/modules/common-attributes.adoc`:
- Line 48: The shared attribute definition is making downstream-only content
render in upstream builds as well. Remove the unconditional :downstream: setting
from common-attributes and set it only from a downstream-specific include or
build argument; this keeps the ifdef::downstream[] sections in the install and
upgrade docs mutually exclusive. Use the downstream attribute handling around
common-attributes and the install/upgrade procedure docs as the key places to
adjust.
🪄 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: 3e75a104-87cf-4b1f-928f-21f19cf57dfb
📒 Files selected for processing (4)
documentation/modules/common-attributes.adocdocumentation/modules/proc_installing-mtv-operator.adocdocumentation/modules/proc_upgrading-mtv-ui.adocdocumentation/upstream-attributes.adoc
| // doc metadata | ||
| :icons: font | ||
| :build: downstream | ||
| :downstream: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Scope :downstream: to downstream-only builds.
Defining :downstream: in the shared attributes file makes it true for upstream renders as well, because both master docs include this file unconditionally. That will cause the new ifdef::downstream[] blocks in the install/upgrade procedures to render alongside the upstream content instead of staying mutually exclusive.
♻️ Suggested fix
-:downstream:Set the attribute from a downstream-only include or build arg instead.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| :downstream: |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@documentation/modules/common-attributes.adoc` at line 48, The shared
attribute definition is making downstream-only content render in upstream builds
as well. Remove the unconditional :downstream: setting from common-attributes
and set it only from a downstream-specific include or build argument; this keeps
the ifdef::downstream[] sections in the install and upgrade docs mutually
exclusive. Use the downstream attribute handling around common-attributes and
the install/upgrade procedure docs as the key places to adjust.
Replace ifeval::["{build}" == "value"] with ifdef::value[] to improve
DITA conversion compatibility. This follows the existing conditional
pattern used for ifdef::web[], ifdef::vmware[], etc.
Changes:
- Add :upstream: and :downstream: standalone attributes in attribute files
- Convert all ifeval statements to ifdef in proc_installing-mtv-operator.adoc
- Convert all ifeval statements to ifdef in proc_upgrading-mtv-ui.adoc
This addresses the 7 ConditionalCode warnings flagged by Vale's
AsciiDocDITA rules as part of the CQA 2.0 pre-migration review.
Signed-off-by: danielclowers <dclowers@redhat.com>
4875d2c to
42a5f96
Compare
|
lgtm |
|
Holding on this PR pending the discussion about upstream/downstream strategy. If we're removing upstream conditionals entirely (per Andy's email), this approach would change. Happy to update once direction is clear 👍 |
2 participants
Version(s):
NA
Issue:
https://issues.redhat.com/browse/DOCPLAN-400
Link to docs preview:
QE review:
Additional information:
Converts
ifeval::["{build}" == "value"]toifdef::value[]for DITA compatibility. Adds:upstream:and:downstream:standalone attributes following existing conditional patterns (ifdef::web[],ifdef::vmware[], etc.). Addresses 7 ConditionalCode warnings flagged by Vale AsciiDocDITA rules as part of CQA 2.0 pre-migration review.Summary by CodeRabbit
New Features
Bug Fixes