Message classes#166
Merged
Merged
Conversation
That is even for me :)
The target report-coverage is not enough because sometimes the coverage data are not regenerated and then you do not get tests executed upon `make check`.
📝 WalkthroughWalkthroughAdds ADT Message Class support: centralizes ADT lock/unlock helpers, implements Message and MessageClass with per-message locking and editor pushes, exposes CLI commands, adds JSON fileformat converters, supplies fixtures and comprehensive unit tests, and updates docs and contrib guidance. ChangesMessage Class feature implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 7
🧹 Nitpick comments (1)
test/unit/test_sap_adt_messageclass.py (1)
98-102: ⚡ Quick winReplace loop-based test setup with explicit sequential assignments.
These loop blocks violate the repo’s test-style rule and should be expanded into explicit statements for each message mutation/setup.
As per coding guidelines, "
test/unit/**/*.py: Write tests sequentially without loops".Also applies to: 118-122, 189-193
🤖 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 `@test/unit/test_sap_adt_messageclass.py` around lines 98 - 102, The for-loop that mutates message fields (for msg in msgclass.messages: msg.documented = None; msg.lastchangedby = None; msg.lastmodified = None; msg.name = None) must be replaced with explicit sequential assignments for each message in msgclass.messages (e.g., assign to msgclass.messages[0].documented, .lastchangedby, .lastmodified, .name, then do the same for index 1, etc.) so the test is written without loops; apply the same change pattern to the other loop blocks referenced (around lines 118-122 and 189-193) using the concrete message list indices to set each attribute to None individually.
🤖 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 @.claude/skills/add-support-for-new-adt-object-type/SKILL.md:
- Around line 202-203: Fix the two typos in the checklist items: change
"serilization" to "serialization" in the checklist line that reads "ADT object
class has serilization and deserialization implemented and tested" and change
"iterface" to "interface" in the line "Command line iterface documentation
matches output of the command help" so the checklist reads "ADT object class has
serialization and deserialization implemented and tested" and "Command line
interface documentation matches output of the command help".
In `@doc/architecture.md`:
- Around line 34-35: Update the sentence in the architecture doc that reads
"ABAP objects must be locked before they can be modified and the lock must be
release at the end otherwise other users cannot access the object." by
correcting the verb tense: change the word "release" to "released" so it reads
"...the lock must be released at the end..."; locate the exact sentence (starts
with "ABAP objects must be locked") and apply the single-word fix.
In `@doc/commands/messageclass.md`:
- Around line 38-45: Add the language identifier "text" to the fenced code
blocks that contain the example outputs to satisfy markdown linting: update the
block that starts with "Description: Testing messages" (the table block with
"No. | Text | Selfexplanatory") and the block that starts with "Number: 001"
(the single-message block with "Text : Repository not found") by changing their
opening fences from ``` to ```text.
In `@sap/adt/annotations.py`:
- Around line 439-444: The example uses a misspelled class/variable name
"MyItemsContianer" twice; rename both occurrences to "MyItemsContainer" so the
XmlContainer.define call (XmlContainer.define('the-item-element-name', TheItem))
and the XmlNodeProperty usage in class MyAdtObject (items =
XmlNodeProperty('the-xml-container-name', factory=MyItemsContainer))
consistently reference the correctly spelled identifier.
In `@sap/adt/objects.py`:
- Around line 531-533: The check for lock-result uses direct indexing
resp.headers['Content-Type'] which can raise KeyError on malformed responses;
update the lock-response validation (the block around resp.headers access in the
lock handling function in sap/adt/objects.py) to first guard for the presence of
the header (e.g. use resp.headers.get('Content-Type') or an explicit
"'Content-Type' in resp.headers" check) and if it's missing raise a SAPCliError
with a clear message (include context like the uri and resp.text or
resp.headers) so the code never raises KeyError and always surfaces an
SAPCliError-derived exception.
In `@sap/cli/messageclass.py`:
- Around line 117-118: Replace the plain stderr print
(console.printerr(f'Message {args.msgno} not found')) with raising an exception
derived from sap.errors.SAPCliError so the CLI entrypoint can handle it; either
raise sap.errors.SAPCliError(f"Message {args.msgno} not found") directly or
define and raise a specific subclass (e.g., MessageNotFoundError(SAPCliError))
and include args.msgno in the message so callers can present a proper CLI
failure.
In `@test/unit/test_sap_adt_messageclass.py`:
- Around line 233-257: The test currently doesn't trigger the error path or
assert unlock behavior; update test_set_message_unlock_on_error to attach the
failing PUT simulation to the Connection (e.g., set connection.put =
raise_on_put or monkeypatch the Connection.put used by sap.adt.MessageClass),
then call the method under test on sap.adt.MessageClass (message class instance
created with MESSAGE_CLASS_NAME) inside a try/except to swallow the expected
Exception, and finally assert that the Connection saw unlock requests (verify
that an unlock_for_message/unlock_for_class request was queued/sent or that the
mock responses unlock_msg_resp/unlock_class_resp were consumed), removing unused
locals by actually using unlock_msg_resp/unlock_class_resp in the Connection
initialization so static checks pass. Ensure you reference the test method name
test_set_message_unlock_on_error, the raise_on_put helper, sap.adt.MessageClass,
Connection and MESSAGE_CLASS_NAME when locating the code to change.
---
Nitpick comments:
In `@test/unit/test_sap_adt_messageclass.py`:
- Around line 98-102: The for-loop that mutates message fields (for msg in
msgclass.messages: msg.documented = None; msg.lastchangedby = None;
msg.lastmodified = None; msg.name = None) must be replaced with explicit
sequential assignments for each message in msgclass.messages (e.g., assign to
msgclass.messages[0].documented, .lastchangedby, .lastmodified, .name, then do
the same for index 1, etc.) so the test is written without loops; apply the same
change pattern to the other loop blocks referenced (around lines 118-122 and
189-193) using the concrete message list indices to set each attribute to None
individually.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b48fd54c-0580-4eed-a80f-1b23c7173e9f
📒 Files selected for processing (19)
.claude/skills/add-support-for-new-adt-object-type/SKILL.mdAGENTS.mdMakefiledoc/architecture.mddoc/commands.mddoc/commands/messageclass.mdsap/adt/__init__.pysap/adt/annotations.pysap/adt/messageclass.pysap/adt/objects.pysap/cli/__init__.pysap/cli/messageclass.pysap/platform/abap/fileformats/__init__.pysap/platform/abap/fileformats/messageclass.pytest/unit/fixtures_sap_adt_messageclass.pytest/unit/test_sap_adt_messageclass.pytest/unit/test_sap_adt_object.pytest/unit/test_sap_cli_messageclass.pytest/unit/test_sap_platform_abap_fileformats_messageclass.py
Comment on lines
+202
to
+203
| - [ ] ADT object class has serilization and deserialization implemented and tested | ||
| - [ ] Command line iterface documentation matches output of the command help |
There was a problem hiding this comment.
Fix checklist typos to avoid confusion in contributor guidance.
Please correct spelling in checklist items (serilization → serialization, iterface → interface).
🧰 Tools
🪛 LanguageTool
[grammar] ~202-~202: Ensure spelling is correct
Context: ...# Checklist: - [ ] ADT object class has serilization and deserialization implemented and tes...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~203-~203: Use a hyphen to join words.
Context: ...ion implemented and tested - [ ] Command line iterface documentation matches outp...
(QB_NEW_EN_HYPHEN)
[grammar] ~203-~203: Ensure spelling is correct
Context: ...plemented and tested - [ ] Command line iterface documentation matches output of the com...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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 @.claude/skills/add-support-for-new-adt-object-type/SKILL.md around lines 202
- 203, Fix the two typos in the checklist items: change "serilization" to
"serialization" in the checklist line that reads "ADT object class has
serilization and deserialization implemented and tested" and change "iterface"
to "interface" in the line "Command line iterface documentation matches output
of the command help" so the checklist reads "ADT object class has serialization
and deserialization implemented and tested" and "Command line interface
documentation matches output of the command help".
Comment on lines
+34
to
+35
| ABAP objects must be locked before they can be modified and the lock must be | ||
| release at the end otherwise other users cannot access the object. |
There was a problem hiding this comment.
Fix verb tense in lock lifecycle description.
Use “the lock must be released at the end” instead of “release” for grammatical correctness.
🤖 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 `@doc/architecture.md` around lines 34 - 35, Update the sentence in the
architecture doc that reads "ABAP objects must be locked before they can be
modified and the lock must be release at the end otherwise other users cannot
access the object." by correcting the verb tense: change the word "release" to
"released" so it reads "...the lock must be released at the end..."; locate the
exact sentence (starts with "ABAP objects must be locked") and apply the
single-word fix.
Comment on lines
+38
to
+45
| ``` | ||
| Description: Testing messages | ||
|
|
||
| No. | Text | Selfexplanatory | ||
| ----|--------------------|----------------- | ||
| 000 | & | true | ||
| 001 | Repository not found | true | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
Two example output blocks are missing fence languages; add text to satisfy markdown linting and improve rendering consistency.
Proposed diff
-```
+```text
Description: Testing messages
No. | Text | Selfexplanatory
----|--------------------|-----------------
000 | & | true
001 | Repository not found | true@@
- +text
Number: 001
Text : Repository not found
S./Ex.: true
Also applies to: 106-110
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 38-38: 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 `@doc/commands/messageclass.md` around lines 38 - 45, Add the language
identifier "text" to the fenced code blocks that contain the example outputs to
satisfy markdown linting: update the block that starts with "Description:
Testing messages" (the table block with "No. | Text | Selfexplanatory") and the
block that starts with "Number: 001" (the single-message block with "Text :
Repository not found") by changing their opening fences from ``` to ```text.
Comment on lines
+233
to
+257
| def test_set_message_unlock_on_error(self): | ||
| lock_msg_resp = Response( | ||
| text=LOCK_RESPONSE_XML, status_code=200, | ||
| headers={'Content-Type': 'application/vnd.sap.as+xml; charset=utf-8; dataname=com.sap.adt.lock.Result'} | ||
| ) | ||
| lock_class_resp = Response( | ||
| text='<?xml version="1.0" encoding="UTF-8"?><asx:abap xmlns:asx="http://www.sap.com/abapxml" version="1.0"><asx:values><DATA><LOCK_HANDLE>CLASS_LOCK</LOCK_HANDLE></DATA></asx:values></asx:abap>', | ||
| status_code=200, | ||
| headers={'Content-Type': 'application/vnd.sap.as+xml; charset=utf-8; dataname=com.sap.adt.lock.Result'} | ||
| ) | ||
| get_resp = Response(text=MESSAGE_CLASS_ADT_GET_RESPONSE_EMPTY_XML, status_code=200, headers={}) | ||
| unlock_class_resp = Response(text='', status_code=200, headers={}) | ||
| unlock_msg_resp = Response(text='', status_code=200, headers={}) | ||
|
|
||
| connection = Connection([get_resp, lock_msg_resp, lock_class_resp]) | ||
|
|
||
| # Make PUT raise an exception by not providing a response | ||
| def raise_on_put(*args, **kwargs): | ||
| raise Exception('PUT failed') | ||
|
|
||
| msgclass = sap.adt.MessageClass(connection, MESSAGE_CLASS_NAME) | ||
|
|
||
| # We can't easily simulate PUT failure with Connection mock, so we test the normal path | ||
| # The try/finally pattern is tested by code inspection | ||
|
|
There was a problem hiding this comment.
test_set_message_unlock_on_error is currently a no-op and fails linting.
At Line 233 onward, the test neither executes the failure path nor asserts outcomes, and it introduces unused locals (F841 at Lines 244, 245, 253). This leaves unlock-on-error behavior unverified and breaks static checks.
🧰 Tools
🪛 Flake8 (7.3.0)
[error] 244-244: local variable 'unlock_class_resp' is assigned to but never used
(F841)
[error] 245-245: local variable 'unlock_msg_resp' is assigned to but never used
(F841)
[error] 253-253: local variable 'msgclass' is assigned to but never used
(F841)
🤖 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 `@test/unit/test_sap_adt_messageclass.py` around lines 233 - 257, The test
currently doesn't trigger the error path or assert unlock behavior; update
test_set_message_unlock_on_error to attach the failing PUT simulation to the
Connection (e.g., set connection.put = raise_on_put or monkeypatch the
Connection.put used by sap.adt.MessageClass), then call the method under test on
sap.adt.MessageClass (message class instance created with MESSAGE_CLASS_NAME)
inside a try/except to swallow the expected Exception, and finally assert that
the Connection saw unlock requests (verify that an
unlock_for_message/unlock_for_class request was queued/sent or that the mock
responses unlock_msg_resp/unlock_class_resp were consumed), removing unused
locals by actually using unlock_msg_resp/unlock_class_resp in the Connection
initialization so static checks pass. Ensure you reference the test method name
test_set_message_unlock_on_error, the raise_on_put helper, sap.adt.MessageClass,
Connection and MESSAGE_CLASS_NAME when locating the code to change.
For me and for agents - it almost forgot how it works.
I was lazy to paste the complete XML data and the LLM added strange content which I did not review and then I was surprised.
This just covers my latest experience where LLM generated new function for PUT and locking.
Because we need it for Messages of Message Classes. I could also add a solo parser of the response because the Message locking is kinda more complicated and needs some additional attributes but the extra parser for locking response looked like an over engineering to me.
The MessageClass object is used as data but also as pure request crate when deleting a single message (i.e. deletedmessages). I wanted to have MessageClass as public interface inheriting from ADTObject but not having the member deletedmessages. Then I wanted to have _MessageClassImpl which whould hold all the members but would not be avaiable in public interface. The public class MessageClass would have a single member "_message" of the type _MessageClassImpl would overwrite all ADTObject methods and pass the calls to the "_message". However LLM told me it is an overengineering and kinda agree. Yet the API of MessageClass is not nice - fortunately, sapcli is not so focused to provide Python API for ADT. I introduced the MessageLockManager to be able to use "with" and because I plan to add mass message update by loading json mesage class definition and merge it with fetched data. Then MessageLockManager will simplify the operation then.
I do not want to have these file format converters in ADT objects directly and so I think it should be in a solo method outside the ADT sub-module. This is a first version which has pure methods but I future, we may use some kind of object format registry and then we will probably use an object instead.
filak-sap
force-pushed
the
message_classes
branch
from
697376d to
08c615f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
doc/commands/messageclass.md (1)
38-45:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd language identifiers to example output fences.
Both output blocks use plain triple backticks; use
textto satisfy MD040 and keep docs lint-clean.Proposed fix
-``` +```text Description: Testing messages @@ -``` +```text Number: 001Also applies to: 106-110
🤖 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 `@doc/commands/messageclass.md` around lines 38 - 45, The markdown example output fences in doc/commands/messageclass.md are missing language identifiers; replace the opening triple-backtick markers for the example blocks (the block starting with "Description: Testing messages" and the block containing "Number: 001" as well as the other occurrence around lines 106-110) from ``` to ```text so the fenced blocks are explicitly marked as text and satisfy MD040.doc/architecture.md (1)
34-35:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix lock lifecycle grammar.
On Line 35, change “must be release” to “must be released”.
🤖 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 `@doc/architecture.md` around lines 34 - 35, Fix the grammar in the sentence that currently reads "the lock must be release" by changing it to "the lock must be released" in the ABAP locking description; locate the sentence in the same paragraph that begins "ABAP objects must be locked..." and update "must be release" to "must be released".
🤖 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 `@sap/cli/messageclass.py`:
- Around line 3-9: The module references SAPCliError in message_read but never
imports it, causing a NameError; update the imports at the top of
sap/cli/messageclass.py to import SAPCliError (the symbol) from its defining
module so message_read can raise SAPCliError without error; ensure the import is
added alongside the existing imports so linting and runtime will succeed.
In `@test/unit/test_sap_cli_messageclass.py`:
- Around line 10-15: The import list in test_sap_cli_messageclass.py includes
unused fixtures MESSAGE_CLASS_ADT_GET_RESPONSE_EMPTY_XML and LOCK_RESPONSE_XML
causing F401; remove these two symbols from the from-import statement (leaving
MESSAGE_CLASS_NAME and MESSAGE_CLASS_ADT_GET_RESPONSE_XML) or, if they should be
used, reference them in the test functions MESSAGE_CLASS_ADT_GET_RESPONSE_XML or
MESSAGE_CLASS_NAME; update the from fixtures_sap_adt_messageclass import to only
import the actually used names to resolve the linter error.
---
Duplicate comments:
In `@doc/architecture.md`:
- Around line 34-35: Fix the grammar in the sentence that currently reads "the
lock must be release" by changing it to "the lock must be released" in the ABAP
locking description; locate the sentence in the same paragraph that begins "ABAP
objects must be locked..." and update "must be release" to "must be released".
In `@doc/commands/messageclass.md`:
- Around line 38-45: The markdown example output fences in
doc/commands/messageclass.md are missing language identifiers; replace the
opening triple-backtick markers for the example blocks (the block starting with
"Description: Testing messages" and the block containing "Number: 001" as well
as the other occurrence around lines 106-110) from ``` to ```text so the fenced
blocks are explicitly marked as text and satisfy MD040.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 99738901-5e94-4cf9-a937-d9e232b1f369
📒 Files selected for processing (18)
.claude/skills/add-support-for-new-adt-object-type/SKILL.mdAGENTS.mddoc/architecture.mddoc/commands.mddoc/commands/messageclass.mdsap/adt/__init__.pysap/adt/annotations.pysap/adt/messageclass.pysap/adt/objects.pysap/cli/__init__.pysap/cli/messageclass.pysap/platform/abap/fileformats/__init__.pysap/platform/abap/fileformats/messageclass.pytest/unit/fixtures_sap_adt_messageclass.pytest/unit/test_sap_adt_messageclass.pytest/unit/test_sap_adt_object.pytest/unit/test_sap_cli_messageclass.pytest/unit/test_sap_platform_abap_fileformats_messageclass.py
✅ Files skipped from review due to trivial changes (4)
- doc/commands.md
- sap/adt/annotations.py
- sap/cli/init.py
- sap/adt/init.py
🚧 Files skipped from review as they are similar to previous changes (7)
- AGENTS.md
- test/unit/fixtures_sap_adt_messageclass.py
- test/unit/test_sap_platform_abap_fileformats_messageclass.py
- sap/platform/abap/fileformats/messageclass.py
- .claude/skills/add-support-for-new-adt-object-type/SKILL.md
- test/unit/test_sap_adt_object.py
- sap/adt/messageclass.py
Comment on lines
+10
to
+15
| from fixtures_sap_adt_messageclass import ( | ||
| MESSAGE_CLASS_NAME, | ||
| MESSAGE_CLASS_ADT_GET_RESPONSE_XML, | ||
| MESSAGE_CLASS_ADT_GET_RESPONSE_EMPTY_XML, | ||
| LOCK_RESPONSE_XML, | ||
| ) |
There was a problem hiding this comment.
Remove unused fixture imports.
MESSAGE_CLASS_ADT_GET_RESPONSE_EMPTY_XML and LOCK_RESPONSE_XML are imported but never used, which triggers F401.
Proposed fix
from fixtures_sap_adt_messageclass import (
MESSAGE_CLASS_NAME,
MESSAGE_CLASS_ADT_GET_RESPONSE_XML,
- MESSAGE_CLASS_ADT_GET_RESPONSE_EMPTY_XML,
- LOCK_RESPONSE_XML,
)🧰 Tools
🪛 Flake8 (7.3.0)
[error] 10-10: 'fixtures_sap_adt_messageclass.MESSAGE_CLASS_ADT_GET_RESPONSE_EMPTY_XML' imported but unused
(F401)
[error] 10-10: 'fixtures_sap_adt_messageclass.LOCK_RESPONSE_XML' imported but unused
(F401)
🤖 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 `@test/unit/test_sap_cli_messageclass.py` around lines 10 - 15, The import list
in test_sap_cli_messageclass.py includes unused fixtures
MESSAGE_CLASS_ADT_GET_RESPONSE_EMPTY_XML and LOCK_RESPONSE_XML causing F401;
remove these two symbols from the from-import statement (leaving
MESSAGE_CLASS_NAME and MESSAGE_CLASS_ADT_GET_RESPONSE_XML) or, if they should be
used, reference them in the test functions MESSAGE_CLASS_ADT_GET_RESPONSE_XML or
MESSAGE_CLASS_NAME; update the from fixtures_sap_adt_messageclass import to only
import the actually used names to resolve the linter error.
I am not sure what should be the default output for read of message - JSON or HUMAN? LLMs are good at parsing JSON but it consumes more tokens.
filak-sap
force-pushed
the
message_classes
branch
from
08c615f to
88cec05
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
doc/commands/messageclass.md (1)
38-45:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
textlanguage to example output fences.Both output examples should use typed fences to satisfy markdown linting consistency.
Proposed diff
-``` +```text Description: Testing messages No. | Text | Selfexplanatory ----|--------------------|----------------- 000 | & | true 001 | Repository not found | true@@
-+text
Number: 001
Text : Repository not found
S./Ex.: trueAlso applies to: 106-110
🤖 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 `@doc/commands/messageclass.md` around lines 38 - 45, Update the two example output fenced code blocks so they use typed fences instead of plain backticks: replace the starting triple-backtick fences before the blocks that begin with "Description: Testing messages" and "Number: 001" (and their corresponding closing fences) with ```text so both examples use ```text ... ```; ensure both instances (the table block starting with "Description: Testing messages" and the single-message block starting with "Number: 001") are updated to maintain markdown lint consistency.
🤖 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.
Duplicate comments:
In `@doc/commands/messageclass.md`:
- Around line 38-45: Update the two example output fenced code blocks so they
use typed fences instead of plain backticks: replace the starting
triple-backtick fences before the blocks that begin with "Description: Testing
messages" and "Number: 001" (and their corresponding closing fences) with
```text so both examples use ```text ... ```; ensure both instances (the table
block starting with "Description: Testing messages" and the single-message block
starting with "Number: 001") are updated to maintain markdown lint consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 511fd9a1-b818-463e-a144-421521a87c7c
📒 Files selected for processing (5)
doc/commands.mddoc/commands/messageclass.mdsap/cli/__init__.pysap/cli/messageclass.pytest/unit/test_sap_cli_messageclass.py
✅ Files skipped from review due to trivial changes (1)
- doc/commands.md
🚧 Files skipped from review as they are similar to previous changes (2)
- sap/cli/init.py
- sap/cli/messageclass.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.