NO-ISSUE: Update DEVELOPMENT.md file with assisted-installer-app project#2912
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@ammont82: This pull request explicitly references no jira issue. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughThe documentation in Changes
Poem
Suggested labels
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
openshift-ci
bot
added
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/DEVELOPMENT.md (2)
75-85: Ensure Consistency Between Installation Command and Note
The code block now correctly usesnpm installfor setting up the assisted-installer-app, which aligns with the PR objective to switch from Yarn to npm. However, the following note still mentions thatyalc linkneeds to be executed afteryarn install. To avoid user confusion, please update this note to referencenpm installinstead. For example, you might change the note to:-**Note**: `yalc link` needs to be executed following the `yarn install` command. +**Note**: `yalc link` needs to be executed following the `npm install` command.
99-104: Verify Integration Command for UHC-Portal
The instructions now indicate that users should follow the uhc-portal’s README and runyarn start --env ai_standalone. Since the development commands for the assisted-installer-app have migrated to npm, double-check whether the uhc-portal integration should remain on Yarn or also transition to npm. Clear separation or updated guidance here would help users avoid potential confusion.🧰 Tools
🪛 LanguageTool
[style] ~100-~100: Consider a shorter alternative to avoid wordiness.
Context: ...ice/uhc-portal/-/blob/master/README.md) in order to set up their dev-environment. ```bas...(IN_ORDER_TO_PREMIUM)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/DEVELOPMENT.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/DEVELOPMENT.md
[style] ~100-~100: Consider a shorter alternative to avoid wordiness.
Context: ...ice/uhc-portal/-/blob/master/README.md) in order to set up their dev-environment. ```bas...
(IN_ORDER_TO_PREMIUM)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: tests
- GitHub Check: unit-tests
- GitHub Check: circular-deps
- GitHub Check: lint
- GitHub Check: translation-files
- GitHub Check: format
🔇 Additional comments (4)
docs/DEVELOPMENT.md (4)
69-71: Update Assisted Installer App Clone Instructions
The updated instructions now include forking and cloning the new assisted-installer-app repository (using the correct GitHub URL). Ensure that the<username>placeholder is eventually replaced during actual usage.
73-73: Clarify Assisted Installer App Setup Step
The step explicitly instructs users to execute the commands inside the assisted-installer-app project. This helps distinguish it from other parts of the monorepo.
86-91: Update Start Command for Assisted Installer App
Changing the start command tonpm run start:federatedaccurately reflects the updated project setup. This new command replaces the old Yarn-based command, making the instructions consistent with the transition to npm.
93-97: Review UHC-Portal Cloning Instructions
The instructions for forking and cloning the uhc-portal project are retained. Please verify that these steps remain valid if both assisted-installer-app and uhc-portal integrations continue to be supported. If the focus is shifting exclusively to the assisted-installer-app, consider updating or deprecating this section.
ammont82
force-pushed
the
no-issue-update-development-doc
branch
from
a68ce52 to
cec7c08
Compare
openshift-ci
bot
removed
the
size/S
openshift-ci
bot
added
the
size/M
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
docs/DEVELOPMENT.md (4)
69-72: New Assisted Installer App Cloning Instructions
The new step clearly instructs users to fork and clone the assisted-installer-app project. Verify that the<username>placeholder gets replaced appropriately by contributors.
73-80: New Setup Commands for Assisted Installer App
The code block withnpm installfollowed by theyalc linkcommands for the required packages is clearly presented. Please ensure that switching from Yarn (as used in other parts of the document) to npm in this context is intentional and well understood by users.
92-95: Clarification on UHC-Portal Integration Instructions
The document now includes instructions for both the assisted-installer-app and the uhc-portal. Please confirm that maintaining both sets of instructions is intentional and consider adding a brief explanation or visual separation so users can easily differentiate between setting up the two projects.
100-104: Streamline Language for Clarity
The instruction “in order to set up their dev-environment” might be simplified for brevity and clarity (e.g., “to set up your dev environment”). This small refinement can improve readability.🧰 Tools
🪛 LanguageTool
[style] ~101-~101: Consider a shorter alternative to avoid wordiness.
Context: ...ob/master/README.md#running-locally) in order to set up their dev-environment. ```bas...(IN_ORDER_TO_PREMIUM)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/DEVELOPMENT.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/DEVELOPMENT.md
[style] ~101-~101: Consider a shorter alternative to avoid wordiness.
Context: ...ob/master/README.md#running-locally) in order to set up their dev-environment. ```bas...
(IN_ORDER_TO_PREMIUM)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: translation-files
- GitHub Check: tests
- GitHub Check: lint
- GitHub Check: format
- GitHub Check: circular-deps
- GitHub Check: unit-tests
🔇 Additional comments (1)
docs/DEVELOPMENT.md (1)
86-90: Launching the Assisted Installer App
The new instructions to start the assisted-installer-app project usingnpm run start:federatedare correctly added. Confirm that this command is valid for the current project setup.
rawagner
left a comment
rawagner
left a comment
There was a problem hiding this comment.
a few nits, otherwise looks great
Co-authored-by: Rastislav Wagner <rawagner@redhat.com>
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ammont82, jgyselov, rawagner The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/DEVELOPMENT.md (1)
101-101: Consider shortening "in order to" for conciseness.The phrase "in order to set up their dev-environment" can be shortened to "to set up their dev-environment" for improved readability.
Apply this diff for conciseness:
- [README file](https://github.com/RedHatInsights/uhc-portal/blob/master/README.md#running-locally) - in order to set up their dev-environment. + [README file](https://github.com/RedHatInsights/uhc-portal/blob/master/README.md#running-locally) + to set up their dev-environment.🧰 Tools
🪛 LanguageTool
[style] ~101-~101: Consider a shorter alternative to avoid wordiness.
Context: ...ob/master/README.md#running-locally) in order to set up their dev-environment. ```bas...(IN_ORDER_TO_PREMIUM)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/DEVELOPMENT.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/DEVELOPMENT.md
[style] ~101-~101: Consider a shorter alternative to avoid wordiness.
Context: ...ob/master/README.md#running-locally) in order to set up their dev-environment. ```bas...
(IN_ORDER_TO_PREMIUM)
🔇 Additional comments (1)
docs/DEVELOPMENT.md (1)
69-104: Clear and accurate integration instructions.The revised steps for integrating with
assisted-installer-appanduhc-portalare clear, accurate, and reflect the current project structure. The use ofnpm ciand the explicit note aboutyalc linkare both correct and helpful. All past review suggestions have been addressed.🧰 Tools
🪛 LanguageTool
[style] ~101-~101: Consider a shorter alternative to avoid wordiness.
Context: ...ob/master/README.md#running-locally) in order to set up their dev-environment. ```bas...(IN_ORDER_TO_PREMIUM)
openshift-merge-bot
bot
merged commit fde98bb
into
openshift-assisted:master
…ect (openshift-assisted#2912) * Update DEVELOPMENT.md file with assisted-installer-app project * Apply suggestions from code review Co-authored-by: Rastislav Wagner <rawagner@redhat.com> --------- Co-authored-by: Rastislav Wagner <rawagner@redhat.com>
4 participants
Summary by CodeRabbit
yarntonpmand added guidance for starting the project withnpm run start:federated.