LCORE-1645: Contribution and branching documentation#1937
Conversation
Walkthrough
ChangesBranching Strategy and Git Workflow Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/branching.md (1)
79-81:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftRelease branch naming convention contradicts existing documentation.
The naming example here uses
release/MAJOR.MINOR.PATCH, butdocs/releasing.md(line 183–188) explicitly states the current convention isrelease/<major>.<minor>(e.g.,release/0.5,release/0.6). The patch-level convention is documented as "aspirational and is not currently used by Konflux components or CI pipelines."The shell command examples at lines 281 and 303 also use patch-level naming (
release/1.2.0,release/1.2.1), which would be inconsistent with the established repository practice.Action: Decide whether this PR intentionally introduces a new convention or should align with the existing
release/x.yformat documented indocs/releasing.md. If alignment is needed, update the naming throughout this section.🤖 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 `@docs/branching.md` around lines 79 - 81, The release branch naming convention in docs/branching.md is inconsistent with the established convention documented in docs/releasing.md. The example at lines 79-81 shows release/MAJOR.MINOR.PATCH format, and the shell command examples at lines 281 and 303 use patch-level versions (release/1.2.0, release/1.2.1), but the established convention per docs/releasing.md is release/major.minor format (e.g., release/0.5, release/0.6) without the patch level. Update all release branch naming examples in docs/branching.md to align with the established release/x.y format convention to ensure consistency across documentation.
🤖 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 `@docs/branching.md`:
- Line 15: Standardize the hyphenation of "cherry-picking" throughout the
document. Replace all instances of "Cherry picking" (non-hyphenated) with
"Cherry-picking" (hyphenated) on lines 20 and 250 to match the correct usage on
line 15 in the table of contents. This ensures grammatical correctness and
visual consistency across all headings and references in the branching.md file.
- Line 199: The fenced code block containing the ASCII diagram with the "main
branch" label is missing a language identifier after the opening backticks. Add
the language identifier "text" to the opening fence by changing ``` to ```text
to comply with the MD040 markdown linter rule and enable proper syntax
highlighting.
---
Outside diff comments:
In `@docs/branching.md`:
- Around line 79-81: The release branch naming convention in docs/branching.md
is inconsistent with the established convention documented in docs/releasing.md.
The example at lines 79-81 shows release/MAJOR.MINOR.PATCH format, and the shell
command examples at lines 281 and 303 use patch-level versions (release/1.2.0,
release/1.2.1), but the established convention per docs/releasing.md is
release/major.minor format (e.g., release/0.5, release/0.6) without the patch
level. Update all release branch naming examples in docs/branching.md to align
with the established release/x.y format convention to ensure consistency across
documentation.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 948e7f4e-099e-480c-b263-b313dbdca851
📒 Files selected for processing (1)
docs/branching.md
📜 Review details
⏰ 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). (14)
- GitHub Check: check_dependencies
- GitHub Check: Pylinter
- GitHub Check: radon
- GitHub Check: shellcheck
- GitHub Check: mypy
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-6-on-pull-request
🧰 Additional context used
🪛 LanguageTool
docs/branching.md
[grammar] ~250-~250: Use a hyphen to join words.
Context: ... +-----------------+ ``` ### Cherry picking Cherry-picking is a Git operati...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.22.1)
docs/branching.md
[warning] 199-199: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
docs/branching.md (1)
173-194: LGTM!
| * [Branching visualization](#branching-visualization) | ||
| * [Feature and fix branches that are merged into the main branch](#feature-and-fix-branches-that-are-merged-into-the-main-branch) | ||
| * [Cherry picking changes into release (stable) branch from the main branch](#cherry-picking-changes-into-release-stable-branch-from-the-main-branch) | ||
| * [Cherry-picking changes into release (stable) branch from the main branch](#cherry-picking-changes-into-release-stable-branch-from-the-main-branch) |
There was a problem hiding this comment.
Standardize hyphenation of "cherry-picking" across TOC and headings.
Line 15 uses "Cherry-picking" (hyphenated) in the TOC, but lines 20 and 250 use "Cherry picking" (non-hyphenated). The grammar analysis tool also flags line 250 as needing a hyphen for compound-word consistency.
Recommend standardizing on "Cherry-picking" throughout for grammatical correctness and visual consistency.
✏️ Proposed fix
- * [Cherry picking](`#cherry-picking`)
+ * [Cherry-picking](`#cherry-picking`)
-### Cherry picking
+### Cherry-pickingAlso applies to: 20-20, 250-250
🤖 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 `@docs/branching.md` at line 15, Standardize the hyphenation of
"cherry-picking" throughout the document. Replace all instances of "Cherry
picking" (non-hyphenated) with "Cherry-picking" (hyphenated) on lines 20 and 250
to match the correct usage on line 15 in the table of contents. This ensures
grammatical correctness and visual consistency across all headings and
references in the branching.md file.
Source: Linters/SAST tools
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
gh issue create \
--repo "lightspeed-core/lightspeed-stack" \
--title "docs/branching.md: Standardize hyphenation of \"cherry-picking\" across TOC and headings" \
--body "## Summary
In \`docs/branching.md\`, the term \"cherry-picking\" is hyphenated inconsistently:
- **Line 15** (TOC): \"Cherry-picking\" ✅ (hyphenated — correct)
- **Line 20** (TOC): \"Cherry picking\" ❌ (non-hyphenated)
- **Line 250** (section heading): \"Cherry picking\" ❌ (non-hyphenated)
All occurrences should use the hyphenated form \"Cherry-picking\" for grammatical correctness and visual consistency.
## Suggested fix
\`\`\`diff
- * [Cherry picking](`#cherry-picking`)
+ * [Cherry-picking](`#cherry-picking`)
-### Cherry picking
+### Cherry-picking
\`\`\`
## References
- PR: https://github.com/lightspeed-core/lightspeed-stack/pull/1937
- Review comment: https://github.com/lightspeed-core/lightspeed-stack/pull/1937#discussion_r3426183918
/cc `@tisnik`" \
--assignee "tisnik"Length of output: 1113
@tisnik, the issue has been created and assigned to you: #1939
|
|
||
| ### Visualized flow | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Add language identifier to code block.
The fenced code block lacks a language identifier. Markdown linter (MD040) recommends specifying the language to improve syntax highlighting and accessibility.
🔧 Proposed fix
-```
+```text
+-----------------+
| main branch |📝 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.
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 199-199: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/branching.md` at line 199, The fenced code block containing the ASCII
diagram with the "main branch" label is missing a language identifier after the
opening backticks. Add the language identifier "text" to the opening fence by
changing ``` to ```text to comply with the MD040 markdown linter rule and enable
proper syntax highlighting.
Source: Linters/SAST tools
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
gh issue create \
--repo "lightspeed-core/lightspeed-stack" \
--title "docs/branching.md: fenced code block missing language identifier (MD040)" \
--assignee "tisnik" \
--body "## Summary
The fenced code block containing the ASCII flow diagram in \`docs/branching.md\` at line 199 is missing a language identifier, which violates the MD040 markdownlint rule (\`fenced-code-language\`).
## Details
- **File:** \`docs/branching.md\`
- **Line:** 199
- **Rule:** MD040 – Fenced code blocks should have a language specified
## Suggested Fix
Add the \`text\` language identifier to the opening fence of the ASCII diagram block:
\`\`\`diff
-\`\`\`
+\`\`\`text
+-----------------+
| main branch |
\`\`\`
## References
- PR: https://github.com/lightspeed-core/lightspeed-stack/pull/1937
- Review comment: https://github.com/lightspeed-core/lightspeed-stack/pull/1937#discussion_r3426183928
Reported by `@tisnik`"Length of output: 1098
@tisnik, I've created the follow-up issue: #1940 — assigned to you. It tracks adding the text language identifier to the fenced code block at line 199 of docs/branching.md.
Description
LCORE-1645: Contribution and branching documentation
Type of change
Tools used to create PR
Related Tickets & Documents
Summary by CodeRabbit