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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow include level pragmas #555

Closed
pedrohaccorsi opened this issue Mar 4, 2022 · 4 comments
Closed

allow include level pragmas #555

pedrohaccorsi opened this issue Mar 4, 2022 · 4 comments
Labels
feature New feature or refactoring

Comments

@pedrohaccorsi
Copy link

pedrohaccorsi commented Mar 4, 2022

馃挕 Code pal for ABAP follows the Clean ABAP. If the issue relates to coding style, please submit it here.

Actual Behavior

Currently, we have to set pragmas on FORM-ENDFORM or METHOD-ENDMETHOD level, which means adding multiple pragmas to the same file sometimes.

Expected Behavior

Be able to add a list of pragmas in the file start/end so that the whole include is ignored from code pal checks.

Why
On major refactor of legacy code, one of the steps is to break the infinitely-long legacy includes into a more manageable list of includes, with fewer code. However, these many includes still have the exact same legacy code from the original gigantic include. This means that very old code now gets scanned by our current code quality checks, and teams have to manually add all different pragmas to all different forms/methods, until the moment of the code refactor comes, which may take months or sometimes not even happen.

Example: one old include with 1000 forms, now broken into 10 includes 100 forms each; since they are in new includes and are considered (correctly) new code, code pal scans them and throws, let's say, errors for 10% of the "new code", which means 100 pragmas we must add. This way, having a simple include-level pragma would be much beneficial.

[edit]
The same could be done in report level. For example, recently we had the report RP_HRCCO_NECOH_DEL (system EHR) automatically generated in order to delete entries in a new table we made. This report is mandatory and must be generated automatically, it is a sap-abap thing, not really under our control. Therefore, there should be a way to "turn off" code pal for this report entirely.

@pedrohaccorsi pedrohaccorsi added the bug Something isn't working correctly label Mar 4, 2022
@bjoern-jueliger-sap
Copy link
Member

Why do the teams "have" to add pragmas?

The intention of pragmas/pseudo comments is that developers should place these in cases where the finding of the check is accurate, but they do not intend to fix them (for whatever reason). If you actually agree that these findings should be fixed but you just do not have time to do so right now, the intended way of dealing with that in the ATC is to request exemptions for these findings - you can even set an expiration date for these in order to get reminded of these findings whenever you want. You should be able to request exemptions for the entire object, which is also much quicker than manually adding pseudo comments.

@bjoern-jueliger-sap bjoern-jueliger-sap added feature New feature or refactoring and removed bug Something isn't working correctly labels Mar 16, 2022
@pedrohaccorsi
Copy link
Author

Hi! Thanks for the answer, but your words do not apply.

The errors we're getting are straight from code pal, their not ATCs. For example, number of statemets > 50. How do I request an exception for that, as that isn't an ATC at all? That's a clean code issue...

Si I didn't really understand your proposal.

@bjoern-jueliger-sap
Copy link
Member

I think we're getting confused about terminology here:

The ABAP Test Cockpit (ATC) is a tool to execute automated checks on objects in an ABAP system. The tool itself does not consist of any specific checks, it is just a tool to execute checks.

The CodePAL checks are a set of specific checks that can be executed via ATC (they can also be executed via Code Inspector, but in general there is no reason to use CI directly instead of ATC). So when you execute the CodePAL checks via ATC, their findings become "ATC findings", and the ATC has generic functionality to request and approve exemptions for these findings - precisely because it would be wasted effort if each individual check created their own bespoke solution for this issue.

If you don't know about ATC exemptions, this blog post is a nice introduction (if potentially slightly outdated). Note in particular the option to create exemptions with a scope of "This Object" and "This Check", which should have exactly the same effect as the type of pseudo comment you suggested.

@pedrohaccorsi
Copy link
Author

Oohhh nice, now I understood your point! Very nice, I dindn't know I could do that. Definetely gonna try it out.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or refactoring
Projects
None yet
Development

No branches or pull requests

2 participants