-
Notifications
You must be signed in to change notification settings - Fork 62
LCORE-1175: Refreshed "Run LCS locally" #989
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThree changes update local development setup: Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
scripts/generate_local_run.sh (1)
21-24: Consider checking sed exit status before declaring success.If
sedfails (e.g., due to write permission issues or disk full), the script still prints a success message. This could be misleading to users.Suggested improvement
# Replace ~/ with $HOME/ and write to local-run.yaml -sed "s|~/|$HOME/|g" "$INPUT_FILE" > "$OUTPUT_FILE" - -echo "Successfully generated $OUTPUT_FILE" +if sed "s|~/|$HOME/|g" "$INPUT_FILE" > "$OUTPUT_FILE"; then + echo "Successfully generated $OUTPUT_FILE" +else + echo "Error: Failed to generate $OUTPUT_FILE" >&2 + exit 1 +fiREADME.md (1)
177-180: Add language specifier to the fenced code block.The code block is missing a language identifier, which affects syntax highlighting and consistency with other code blocks in the document.
Suggested fix
6. start LCS server - ``` + ```bash make run ```
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignoreREADME.mdscripts/generate_local_run.sh
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
178-178: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-pr
- GitHub Check: E2E: library mode / ci
- GitHub Check: E2E: server mode / ci
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
🔇 Additional comments (3)
.gitignore (1)
194-196: LGTM!Correctly ignores the generated
local-run.yamlfile which contains user-specific home directory paths and should not be committed to version control.scripts/generate_local_run.sh (1)
1-19: LGTM!Good practices observed:
- Proper shebang and script directory resolution using
BASH_SOURCE- Correct derivation of project root
- Input file existence check with proper error handling to stderr
- Using
|as sed delimiter to avoid conflicts with path separatorsREADME.md (1)
155-176: LGTM!The refreshed documentation is clear and well-structured:
- Logical step ordering starting from dependency installation
- Proper use of fenced code blocks with language specifiers
- Clear explanation of the new
local-run.yamlgeneration workflow- Correct reference to the new script at
./scripts/generate_local_run.sh
|
Hey @tisnik, do you think you could give this a look :) Thanks! |
Description
Updated documentation for getting started with LCS. Added a script to generate a
local-run.yamlthat replaces~/with the users home directory fromrun.yaml.Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
N/A
Summary by CodeRabbit
Release Notes
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.