Feature/140 add support for multilevel list of release notes #142
Conversation
- Slit of README.md, developer chapters moved to DEVELOPER.md. -
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughA new developer guide was introduced, consolidating setup and contribution instructions previously in the README. The README was updated to reference this guide. The Changes
Sequence Diagram(s)sequenceDiagram
participant PR_Body as Pull Request Body
participant Record as Record.get_rls_notes()
participant Output as Release Notes Output
PR_Body->>Record: Provide PR body text
Record->>Record: Iterate lines, skip empty lines
Record->>Record: Detect release notes section
Record->>Record: Extract lines with list markers (support multi-level)
Record->>Output: Return formatted release notes
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for parsing multilevel lists in release notes. It updates the test suite to validate multi-level formatting, improves the release notes parsing logic by skipping empty lines, and refreshes documentation by introducing a new Developer Guide.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/release_notes/model/test_record.py | Updated test functions to validate multilevel release notes formatting. |
| release_notes_generator/model/record.py | Enhanced parsing logic by skipping empty lines for better release notes clarity. |
| README.md | Updated to point to the new Developer Guide instead of outdated sections. |
| DEVELOPER.md | Added a new Developer Guide with detailed instructions for developers. |
![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 (1)
DEVELOPER.md (1)
1-181: Great addition of a comprehensive developer guide.This new document consolidates all development-related instructions into a single, well-structured reference. The guide covers all necessary aspects of the development workflow including setup, code quality tools, testing, and local action execution.
A few minor improvements to consider:
- Line 30: Consider "...keep our code quality high with a score above 9.5" for better readability
- Line 60: Add a comma after "In this project"
- Line 98-104: Consider adding a comma after "tool" and "the" before "
pyproject.tomlfile"- Line 135: Consider adding "the" before "pytest-cov coverage tool"
- Line 90: Specify a language for the fenced code block (e.g., "```text")
🧰 Tools
🪛 LanguageTool
[uncategorized] ~30-~30: Possible missing preposition found.
Context: ...p our code quality high above the score 9.5. ### Set Up Python Environment ```shel...(AI_EN_LECTOR_MISSING_PREPOSITION)
[grammar] ~41-~41: This phrase is duplicated. You should probably use “Run Pylint” only once.
Context: ...isted in the project requirements. ### Run Pylint Run Pylint on all files that are currently tracked...(PHRASE_REPETITION)
[grammar] ~59-~59: The verb form ‘root’ does not appear to fit in this context.
Context: ...iewed as a strict subset of PEP 8. The project root filepyproject.tomldefines the Black...(SINGULAR_NOUN_VERB_AGREEMENT)
[typographical] ~60-~60: It appears that a comma is missing.
Context: ...s the Black tool configuration. In this project we are accepting the line length of 120...(DURING_THAT_TIME_COMMA)
[grammar] ~75-~75: This phrase is duplicated. You should probably use “Run Black” only once.
Context: ...isted in the project requirements. ### Run Black Run Black on all files that are currently tracked...(PHRASE_REPETITION)
[uncategorized] ~98-~98: A comma might be missing here.
Context: ...https://mypy.readthedocs.io/en/stable/) tool which is a static type checker for Pyth...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~104-~104: You might be missing the article “the” here.
Context: ...incorrectly. my[py] configuration is inpyproject.tomlfile. Follow these step...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~135-~135: You might be missing the article “the” here.
Context: ...erage Code coverage is collected using pytest-cov coverage tool. To run the tests and...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
90-90: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
DEVELOPER.md(1 hunks)README.md(3 hunks)release_notes_generator/model/record.py(1 hunks)tests/release_notes/model/test_record.py(1 hunks)
🧰 Additional context used
🪛 LanguageTool
DEVELOPER.md
[uncategorized] ~30-~30: Possible missing preposition found.
Context: ...p our code quality high above the score 9.5. ### Set Up Python Environment ```shel...
(AI_EN_LECTOR_MISSING_PREPOSITION)
[grammar] ~41-~41: This phrase is duplicated. You should probably use “Run Pylint” only once.
Context: ...isted in the project requirements. ### Run Pylint Run Pylint on all files that are currently tracked...
(PHRASE_REPETITION)
[grammar] ~59-~59: The verb form ‘root’ does not appear to fit in this context.
Context: ...iewed as a strict subset of PEP 8. The project root file pyproject.toml defines the Black...
(SINGULAR_NOUN_VERB_AGREEMENT)
[typographical] ~60-~60: It appears that a comma is missing.
Context: ...s the Black tool configuration. In this project we are accepting the line length of 120...
(DURING_THAT_TIME_COMMA)
[grammar] ~75-~75: This phrase is duplicated. You should probably use “Run Black” only once.
Context: ...isted in the project requirements. ### Run Black Run Black on all files that are currently tracked...
(PHRASE_REPETITION)
[uncategorized] ~98-~98: A comma might be missing here.
Context: ...https://mypy.readthedocs.io/en/stable/) tool which is a static type checker for Pyth...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~104-~104: You might be missing the article “the” here.
Context: ...incorrectly. my[py] configuration is in pyproject.toml file. Follow these step...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~135-~135: You might be missing the article “the” here.
Context: ...erage Code coverage is collected using pytest-cov coverage tool. To run the tests and...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
DEVELOPER.md
90-90: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (4)
release_notes_generator/model/record.py (1)
157-161: Excellent enhancement for handling empty lines in release notes.This change correctly addresses the requirement to support multilevel lists by skipping empty lines when processing release notes. Empty lines between list items are common in markdown formatting, especially with nested lists, and this change ensures they don't interrupt the extraction process.
README.md (1)
19-19: Good restructuring of documentation.Relocating the detailed developer setup instructions to a dedicated DEVELOPER.md file makes the README more focused and user-friendly. This change properly maintains the table of contents structure while simplifying the main documentation.
Also applies to: 270-272
tests/release_notes/model/test_record.py (2)
69-86: Well-designed test for multilevel list extraction.This test thoroughly verifies the new functionality for handling multilevel lists in release notes. It includes:
- Multiple indentation levels
- Various list markers (-, +, *)
- Empty lines between list items
- Proper termination at a second chapter heading
The test provides good coverage for the updated functionality in the
get_rls_notesmethod.
87-88: Good addition of mixed line marks test.This test complements the multilevel test by specifically focusing on the ability to handle different line markers in a simpler indentation structure. Together with the previous test, this provides comprehensive coverage of the feature.
miroslavpojer
requested review from
Copilot and
OlivieFranklova
and removed request for
Zejnilovic
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for parsing multi-level release note lists, skips empty lines during parsing, and centralizes developer setup instructions.
- Skip empty lines in
get_rls_notesto avoid blank entries - Add and update tests for multi-level release note parsing
- Extract developer setup/docs into
DEVELOPER.md, update README and templates
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/release_notes/model/test_record.py | Added multi-level list tests and renamed test for clarity |
| release_notes_generator/model/record.py | Skip empty lines when parsing release notes body |
| README.md | Updated nested list examples and linked to new Developer Guide |
| DEVELOPER.md | Introduced standalone developer guide with setup and tooling steps |
| .github/workflows/check_pr_release_notes.yml | Extended title regex to accept alternate summary patterns |
| .github/pull_request_template.md | Simplified Release Notes placeholder in PR template |
Comments suppressed due to low confidence (1)
release_notes_generator/model/record.py:162
- Leading whitespace prevents matching the release notes header. Consider using
detection_regex.search(stripped_line)to match against the trimmed line.
if detection_regex.search(line): # Use regex search
| @@ -66,7 +66,25 @@ def test_record_properties_authors_contributors(record_with_no_issue_one_pull_cl | |||
| # get_rls_notes | |||
|
|
|||
|
|
|||
| def test_get_rls_notes(record_with_no_issue_one_pull_closed): | |||
| def test_get_rls_notes_multi_level(record_with_no_issue_one_pull_closed): | |||
| record_with_no_issue_one_pull_closed.pull_request(0).body = """"Release Notes: | |||
There was a problem hiding this comment.
The triple-quoted string literal uses four double quotes, causing a syntax error. It should use three quotes ("""").
| record_with_no_issue_one_pull_closed.pull_request(0).body = """"Release Notes: | |
| record_with_no_issue_one_pull_closed.pull_request(0).body = """Release Notes: |
Copilot uses AI. Check for mistakes.
.github/pull_request_template.md
Outdated
| {PR summary} | ||
|
|
There was a problem hiding this comment.
[nitpick] The placeholder {PR summary} may not render clearly in Markdown; consider providing a proper header or explicit instructions for filling out release notes.
| {PR summary} | |
| ## PR Summary | |
| > Provide a concise summary of the changes introduced in this pull request. |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
DEVELOPER.md (7)
3-8: Standardize table of contents entries. Some link texts (e.g., "Run Unit Test", "Run Static Code Analysis") differ from their section headings ("Running Unit Test", "Running Static Code Analysis"). Consider unifying wording in the TOC to exactly match the headers for consistency and clearer navigation.
39-43: Avoid duplicated phrasing for Pylint execution. The sentence "This command will also install a Pylint tool…" and the heading "### Run Pylint" are very similar. Consider consolidating or renaming the section to eliminate redundancy (e.g., use "### Execute Pylint" or adjust the preceding sentence).🧰 Tools
🪛 LanguageTool
[grammar] ~41-~41: This phrase is duplicated. You should probably use “Run Pylint” only once.
Context: ...isted in the project requirements. ### Run Pylint Run Pylint on all files currently tracked by Git i...(PHRASE_REPETITION)
58-60: Clarify phrasing of project root file. The sentence "The project root filepyproject.tomldefines the Black tool configuration." could be clearer if reworded, for example:The root project file `pyproject.toml` defines the Black configuration.🧰 Tools
🪛 LanguageTool
[grammar] ~59-~59: The verb form ‘root’ does not appear to fit in this context.
Context: ...iewed as a strict subset of PEP 8. The project root filepyproject.tomldefines the Black...(SINGULAR_NOUN_VERB_AGREEMENT)
71-74: Avoid duplicated phrasing for Black execution. Similar to the Pylint section, the sentence "This command will also install a Black tool…" overlaps with the heading "### Run Black". Consider renaming to "### Execute Black" or merging the description to reduce repetition.
89-92: Specify a language for the fenced code block. The expected output example uses a generic code fence without a language. For better readability and lint compliance, specify a language or label, for example:All done! ✨ 🍰 ✨ 1 file reformatted.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
90-90: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
97-97: Fix link text for mypy tool. The label[my[py]]is confusing due to nested brackets. Use[mypy]to match the official tool name:This project uses the [mypy](https://mypy.readthedocs.io/en/stable/) tool…
102-102: Add missing article in mypy configuration description. Changemy[py] configuration is in `pyproject.toml` file.to
mypy configuration is in the `pyproject.toml` file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/pull_request_template.md(1 hunks).github/workflows/check_pr_release_notes.yml(1 hunks)DEVELOPER.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/pull_request_template.md
🧰 Additional context used
🪛 LanguageTool
DEVELOPER.md
[grammar] ~41-~41: This phrase is duplicated. You should probably use “Run Pylint” only once.
Context: ...isted in the project requirements. ### Run Pylint Run Pylint on all files currently tracked by Git i...
(PHRASE_REPETITION)
[grammar] ~59-~59: The verb form ‘root’ does not appear to fit in this context.
Context: ...iewed as a strict subset of PEP 8. The project root file pyproject.toml defines the Black...
(SINGULAR_NOUN_VERB_AGREEMENT)
[grammar] ~75-~75: This phrase is duplicated. You should probably use “Run Black” only once.
Context: ...isted in the project requirements. ### Run Black Run Black on all files currently tracked by Git i...
(PHRASE_REPETITION)
[uncategorized] ~103-~103: You might be missing the article “the” here.
Context: ...incorrectly. my[py] configuration is in pyproject.toml file. Follow these step...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
DEVELOPER.md
90-90: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (1)
.github/workflows/check_pr_release_notes.yml (1)
41-41: Approve updated PR title regex. The pattern correctly expands detection criteria to include both case-insensitive "Release Notes:" and the exact phrase "Summary by CodeRabbit".
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for handling multilevel release notes, including improved parsing of release note entries and an updated developer guide.
- Updated test cases in tests/release_notes/model/test_record.py to verify multilevel release notes.
- Modified parsing logic in release_notes_generator/model/record.py to skip empty lines.
- Updated documentation in README.md, added DEVELOPER.md, and adjusted GitHub workflow and pull request template files.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/release_notes/model/test_record.py | Added tests for multilevel release notes and mixed line mark scenarios |
| release_notes_generator/model/record.py | Skips empty lines during release notes parsing |
| README.md | Updated reference from "Get Started" to "Developer Guide" |
| DEVELOPER.md | Added comprehensive developer guide documentation |
| .github/workflows/check_pr_release_notes.yml | Updated title regex for detecting release notes |
| .github/pull_request_template.md | Updated template with simplified PR summary placeholder |
| - Improved performance | ||
| + Now it runs like a cheetah - really! | ||
|
|
||
| + More nice code |
There was a problem hiding this comment.
The line contains an extra '+' causing the line content to be '++ More nice code', but the expected output requires a single '+'. Please remove the extra '+' for consistency.
Copilot uses AI. Check for mistakes.
| stripped_line = line.strip() | ||
| if len(stripped_line) == 0: | ||
| # skip empty lines as they are not relevant | ||
| continue | ||
|
|
There was a problem hiding this comment.
I don't mind it like this, but we can simplify that
| stripped_line = line.strip() | |
| if len(stripped_line) == 0: | |
| # skip empty lines as they are not relevant | |
| continue | |
| if not line.strip(): | |
| continue # skip empty lines as they are not relevant |
There was a problem hiding this comment.
Nice catch. Fixed.
miroslavpojer
deleted the
feature/140-Add-support-for-multilevel-list-of-release-notes
branch
Summary by CodeRabbit
Closes #140