Skip to Content
Technical Articles

Peer reviewing ABAP code

At work me and my colleagues have been peer reviewing each other’s code for a while in an effort to improve code quality and adopt devops best practices

I find this to be ridiculously painful in ABAP systems, where objects are versioned individually.

We plan to sort this once and for all with ABAPGIT, perhapsĀ  integrate it with a CI tool like jenkins. Or at least rely on an amazing selection of GIT related tools, like gitlens for vs code(below, comparing 2 versions across 10 commits)

Sadly we aren’t ready for that yet, if nothing because we didn’t have time to test it yet (I don’t think we need much time for that, but there’s always something more pressing)

So we are stuck with SAPGUI or Eclipse:

(ok, eclipse is a bit better as shows class interface and methods as a single file)

So since I’m developing my own editor plugin I bit the bullet and added a feature to collect all the changes in a transport and expose them in code’s SCM view. One click and it will show the diff between the last version in a transport and the last version in a previous one.

It’s not solving very well the case when a change has several subsequent changes but it’s a start.

Another source of frustration is telling meaningful changes from meaningless ones. The main problem here is that ABAP is case insensitive, and while we do standardise on lower/uppercase usage, we don’t always do it consistently, and anyway most of our codebase was written before that.

This is what I see after copying an abapgit class, making a few trivial changes and saving (which also auto-formats the source code):

So I added a button to filter out trivial changes (converts to lowercase all but strings and comments)

These features are brand new, I’ve been using the first half for a week and finished the second this morning. Wouldn’t be surprised if there were some issues

 

As I said, the real solution to this problem is tracking your ABAP code in GIT and use a CI tool to manage pull requests. But in the meanwhile this will make my life a little easier

4 Comments
You must be Logged on to comment or reply to a post.
  • Just to add a little detail, in Eclipse, the version comparison of classes shows the differences per section. So you have class definition and implementation in one section, but in another the local classes, and in another the test classes etc. etc. One section for each tab in the class editor.

    • So does this, which beats checking every section and method individually by a mile

      Still like 5 clicks to go from one object to the next, vs one in this (once you prepared it)

  • do you exclude changes to comments only? sometimes developers in a rush to make it work just focus on code and then tidy up with comments. Usually ends up flagging as a change.

    • I don’t touch comments (and strings) at all.

      I convert most of the rest to lower case, including string templates

      The idea is to ignore changes made by the pretty printer and focus on real ones