Skip to content

[DOMA] add longtexts#743

Merged
larshp merged 29 commits intomainfrom
hvam/doma1303
Apr 20, 2026
Merged

[DOMA] add longtexts#743
larshp merged 29 commits intomainfrom
hvam/doma1303

Conversation

@larshp
Copy link
Copy Markdown
Collaborator

@larshp larshp commented Mar 13, 2026

@larshp
Copy link
Copy Markdown
Collaborator Author

larshp commented Mar 13, 2026

regarding compatibility check, I think the change is compatible, its a new optional field

@larshp larshp marked this pull request as ready for review March 13, 2026 10:47
@larshp larshp changed the title DOMA: add longtexts [DOMA] add longtexts Mar 13, 2026
@schneidermic0
Copy link
Copy Markdown
Contributor

About SAPScript long texts already some discussion started. I am afraid I guess we need some time to resolve it.

I try to find some time to involve corresponding colleagues next week. Please be a bit patient with me.

@larshp
Copy link
Copy Markdown
Collaborator Author

larshp commented Mar 20, 2026

About SAPScript long texts already some discussion started. I am afraid I guess we need some time to resolve it.

I try to find some time to involve corresponding colleagues next week. Please be a bit patient with me.

okay, we kind of need it to move forward with abapGit/abapGit#7606, in order to not loose data

@schneidermic0
Copy link
Copy Markdown
Contributor

I have pinged the colleagues.

@larshp
Copy link
Copy Markdown
Collaborator Author

larshp commented Mar 30, 2026

I think lets move forward

#130 was opened 5 years ago

@schneidermic0 schneidermic0 self-assigned this Apr 8, 2026
@schneidermic0
Copy link
Copy Markdown
Contributor

I am sorry for the late response. I have already got feedback by the colleagues some days ago but I didn't manage to answer here, earlier.

There is a team working on another, more readable representation for SAP Script documentation, currently. I cannot share more details on this for now.

I see two options for going forward for the integration in abapGit:

  1. Use the proposal in this PR, for going forward and adapt it to the new representation as soon as it is available. This requires further adaptations (and maybe, compatibility handling).
  2. Don't change the format for documentation for domains (and further objects) and stick to the existing abapGit representation. Then adapt to the new format as soon as it is available.

In any case, I would suggest to move the documentation to a dedicated file instead of incorporating it into the existing JSON representation.

What do you think?

@larshp
Copy link
Copy Markdown
Collaborator Author

larshp commented Apr 9, 2026

-> single file as json will work for now -> PR needs a bit of rework and defining some structure

@larshp larshp marked this pull request as draft April 9, 2026 12:34
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 11, 2026

ABAP Doc Checks

  • Run ABAP Doc title/description casing check

New commits with ABAP changes were pushed. Check the box to run again.

@larshp larshp marked this pull request as ready for review April 11, 2026 09:33
@larshp larshp requested a review from schneidermic0 April 13, 2026 16:25
Copy link
Copy Markdown
Contributor

@schneidermic0 schneidermic0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for splitting this up into a dedicated file. This make sense to me.

Format in general also looks good to me. I have just some naming proposals to avoid "longtexts".

@@ -0,0 +1,33 @@
"! <p class="shorttext synchronized" lang="en">Long text types</p>
"! Types for AFF long texts
INTERFACE zif_aff_longtexts_types_v1 PUBLIC.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LIMU DOCU could end up in:

Suggested change
INTERFACE zif_aff_longtexts_types_v1 PUBLIC.
INTERFACE zif_aff_docu_v1 PUBLIC.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Comment thread file-formats/doma/README.md Outdated
File | Cardinality | Definition | Schema | Example
:--- | :--- | :--- | :--- | :---
`<name>.doma.json` | 1 | [`zif_aff_doma_v1.intf.abap`](./type/zif_aff_doma_v1.intf.abap) | [`doma-v1.json`](./doma-v1.json) | [`z_aff_example.doma.json`](./examples/z_aff_example.doma.json)
`<name>.doma.longtext.json` | 0...1 | [`zif_aff_longtexts_types_v1.intf.abap`](../zif_aff_longtexts_types_v1.intf.abap) | [`longtexts-v1.json`](../longtexts-v1.json) | [`z_aff_example.doma.longtext.json`](./examples/z_aff_example.doma.longtext.json)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a minor: And for sure we can debate about this: I suggest to use either "documentation" or just "docu" instead of "longtext". This fits also better to the transport object LIMU DOCU. Longtext might to fit for domains for others I believe docu(mentation) would be better.

What do you think?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

@larshp
Copy link
Copy Markdown
Collaborator Author

larshp commented Apr 17, 2026

@schneidermic0 comments above addressed

Copy link
Copy Markdown
Contributor

@schneidermic0 schneidermic0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I missed the update. Thanks for pinging me.

I just find a very small thing that I missed before. I try to include the suggested changes right away.

Comment thread file-formats/zif_aff_docu_v1.intf.abap Outdated
Comment thread file-formats/docu-v1.json Outdated
Comment thread file-formats/doma/README.md
Comment thread file-formats/docu-v1.json
@larshp
Copy link
Copy Markdown
Collaborator Author

larshp commented Apr 20, 2026

@schneidermic0 new changes require another approval, looks like merges does that too

Copy link
Copy Markdown
Contributor

@albertmink albertmink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@larshp larshp merged commit 907d44e into main Apr 20, 2026
11 checks passed
@larshp larshp deleted the hvam/doma1303 branch April 20, 2026 15:51
@schneidermic0
Copy link
Copy Markdown
Contributor

new changes require another approval, looks like merges does that too

I didn't find a setting for the automatic merges. Do you know where this can be specified?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants