Conversation
|
|
schneidermic0
left a comment
There was a problem hiding this comment.
Thanks @anne-hobbs for your contribution. 👍
In a first iteration I went only over the example file and added some suggestions/ideas/questions there.
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
Co-authored-by: Michael Schneider <micha.schneider@sap.com>
Co-authored-by: Michael Schneider <micha.schneider@sap.com>
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
Co-authored-by: Katharina Wurz <katharina.wurz@sap.com>
Co-authored-by: Katharina Wurz <katharina.wurz@sap.com>
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
wurzka
left a comment
There was a problem hiding this comment.
I found one issue, but I'm not sure whether this is related to the failing check
Co-authored-by: Katharina Wurz <katharina.wurz@sap.com>
🤖 Generated with Claude Code
|
@anne-hobbs f7d1d5f adds the missing metadata file for the ABAP interface |
|
@larshp just wanted to let you know that the JSON Schema generation looks off. Maybe you know more? |
|
yea, its fixed, with the commit above updating the branch |
wurzka
left a comment
There was a problem hiding this comment.
@anne-hobbs, thanks for the adaption. My review now is based on the structure of the AFF & mandatory fields. Please see my comments.
And @larshp, thanks for fixing 🙏
| "! <p class="shorttext">Parameter Name</p> | ||
| "! Parameter name | ||
| name TYPE ty_field_name, | ||
| "! <p class="shorttext">Table</p> | ||
| "! Table | ||
| table TYPE zif_aff_types_v1=>ty_object_name_30, | ||
| "! <p class="shorttext">Field</p> | ||
| "! Field | ||
| field TYPE ty_field_name, |
There was a problem hiding this comment.
Should anything be required here? So for a lock parameter, is anything field of them mandatory to be filled?
There was a problem hiding this comment.
Hard to say. The lock parameter candidates are always derived from the primary keys of the tables and thus determined by the framework. The only thing that can actually be changed is the name and if it's active or not. Would you recommend setting all three (field, table, name) as required or just the name as it's the only changable part here?
There was a problem hiding this comment.
I think it depends on what is a correct JSON. I understand that name, table and field must be filled, i.e.
{ "name": "bla" }would not be valid, right? Therefore, I tend to also make table and field required.
One other question since I just compare the ADT editor with the JSON here: What does active mean? Is it meant that if active true, then the field is a lock parameter? If yes, I'm wondering if we really need the field and if it isn't enough to be a part of the table lockParameters for being a "lock parameter".
There was a problem hiding this comment.
Active in this case means that it is included in the generation of the lock modules and part of the locking mechanism. All entries in the "lockParameter" section are derived from primary keys and cannot be removed (or others manually added) so this flag is necessary (and besides the name the only one thats actually editable by the user)
There was a problem hiding this comment.
If the "lockParameter" entries are derived from the primary key, are they transported with the lock object?
What does a "lockParameter" with active = false mean?
There was a problem hiding this comment.
Yes, they are as the user has to name them (I don't know why though) and decide if they are going to be considered for the lock module or not.
If a lock parameter is set to active = false then it means that the entries for that table (whose key it is) are not going to be locked when the corresponding entry in the primary table gets locked. You could consider it an "off" switch so to say.
There was a problem hiding this comment.
Does the user also have to name them for active = false?
ABAP Doc Checks
|
AFF definition for Lock Objects (ENQU)