Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consolidate READ/INSERT/DELETE REPORT statements #6299

Merged
merged 10 commits into from Jun 24, 2023
Merged

Conversation

mbtools
Copy link
Member

@mbtools mbtools commented May 21, 2023

Preparation for #6154, no functional changes

Add class zcl_abapgit_sap_report to handle all situations dealing directly with report sources. This provides a central place to handle ABAP language version.

Includes factory/injector to facilitate testing and open-abap.

Ref #6298

Tests:

image
image
image

Preparation for #6154, no functional changes

Add class `zcl_abapgit_sap_report` to handle all situations dealing directly with report sources. This provides a central place to handle ABAP language version.

Includes factory/injector to facilitate testing and open-abap.

Ref #6298
@mbtools mbtools added serialization Translate between object types and files refactoring Change code without altering functionality labels May 21, 2023
@mbtools
Copy link
Member Author

mbtools commented May 25, 2023

Is there a way to exclude zcl_abapgit_sap_report from the tests, @larshp?

@mbtools mbtools marked this pull request as ready for review June 1, 2023 16:21
@larshp
Copy link
Member

larshp commented Jun 23, 2023

will add scaffolding for the statement in abaplint/transpiler#1250, so it compiles the statement, but fails if the unit tests hits it

@mbtools
Copy link
Member Author

mbtools commented Jun 23, 2023

I replaced all remaining programm with syrepid

@larshp
Copy link
Member

larshp commented Jun 23, 2023

nice, unit tests now passing

@mbtools mbtools merged commit c8a4a54 into main Jun 24, 2023
6 checks passed
@mbtools mbtools deleted the mbtools/cons_sap_report branch June 24, 2023 11:20
@chrassig
Copy link

We encountered syntax errors after pulling the current version of abapGit in a system with ABAP version 750 and transporting abapGit to a system with ABAP version 740 (a Solution Manager system, to my knowledge, there is no release of Solution Manager with an ABAP version newer than 740).

Program ZCL_ABAPGIT_SAP_REPORT========CP, Include ZCL_ABAPGIT_SAP_REPORT========CM004: Syntax error in line 000020
Unable to interpret 'VERSION'. Possible causes oferror: Incorrect spelling or comma error

In ABAP release 7.40, the statement INSERT REPORT does not support the addition VERSION that is now used in class ZCL_ABAPGIT_SAP_REPORT of abapGit:

For this reason, the addition UNICODE ENABLING of the statement INSERT REPORT has been replaced by the universal addition VERSION and is now obsolete"

https://help.sap.com/doc/abapdocu_750_index_htm/7.50/en-US/abennews-750-abap_versions.htm#!ABAP_MODIFICATION_2@2@

Is it still the goal of abapGit to support ABAP versions as old as 702? Or will we have to look for a different update and transport strategy for abapGit in our SAP system landscape in the future, as support for older releases will be dropped?

I was able to get a version of abapGit without syntax errors by switching back to tag v1.124.0.

@chrassig
Copy link

I have been looking for a few minutes for a syntax that would allow systems with newer ABAP version (750 or higher) to use the INSERT REPORT statement with the VERSION addition, while enabling systems with older ABAP versions (lower than 750) to still import abapGit via a transport request without running into syntax check.

For example, I was looking for a function module or global class in SAP coding that encapsulates the INSERT REPORT statement including the VERSION addition. We could call a function module or global class pseudo-dynamically if it exists.

Unfortunately, I did not find a matching function module or a class yet, and I do not have a different working idea yet.

@larshp
Copy link
Member

larshp commented Jun 28, 2023

@chrassig please open a new issue

mbtools added a commit that referenced this pull request Jun 28, 2023
Lower releases do not support `version` addition for `insert report` (see [docs](https://help.sap.com/doc/abapdocu_latest_index_htm/latest/en-US/index.htm?file=abapinsert_report.htm#!ABAP_ADDITION_5@5@)). For the cases where a `insert report` is required, abapGit will rely on default system behavior which sets version = X. 

It's probably one of the cases where we need a 750 code base and automatic downport. 🤷 

Ref #6299 (comment)
@chrassig
Copy link

I saw that Marc (@mbtools) already changed the code. The VERSION addition is not used any more.

@larshp, do you still want me to open a new issue?

Thanks a lot to both of you!

@larshp
Copy link
Member

larshp commented Jun 28, 2023

no, but for future reference, its really difficult to keep track of issues inside closed issues/PRs...

@chrassig
Copy link

I agree. With my first comment, I just meant to ask what to do.

I am extremely grateful that you guys invest your time into this project, and I want to try to do whatever I can to contribute and cause you as little work as possible.

So opening an issue would not be a problem for me at all. I just had the impression that it might do more harm than good to open an issue for a problem that has already been fixed.

@mbtools
Copy link
Member Author

mbtools commented Jun 29, 2023

If something isn't working, issues are the way to go since they document it for others (including your future self).

If you or your company really feel bad about it (or love abapGit so much), you can sponsor us 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Change code without altering functionality serialization Translate between object types and files
Development

Successfully merging this pull request may close these issues.

None yet

3 participants