Skip to Content
Technical Articles

How we do ABAP code review using abapGit

We need a way to do code review for our ABAP code, and we decided to give abapGit a go.

 

We have been looking for ways to enforce code review before the transport can be released in our on premise ABAP systems. We can’t find a standard SAP way, but the rest of the world seems to happily use git. Hence we started looking at how to use the notorious abapGit git client for this purpose. We’ve come across a blog by Eduardo Copat that suggested this idea, and this blog is about how we tried to productionise the approach with a few twists.

Our understanding is that abapGit is intended for a different purpose, therefore we expected some compatibility issues with our requirements.

At a very high level, what we have done is use abapGit to serialize our entire code base in production and push it to our internal corporate Github repository. We then create a new repository in abapGit in our development environment that points to the same github repository. (They will be out of sync showing lots of differences but it doesn’t really matter)

Story time….

A developer completes development and is ready to release their transport. He releases all the subtasks of a transport (the transport is not yet released). At this point we have some custom code to automatically create a new branch in github in the background with the same name as the transport. (this piggy backs “Transport to branch” feature in abapGit). We also added a check so that the transport cannot be released without an approved pull request.

(As the tasks are released, we intend to trigger ATC to check for any glaring problems before a code review occurs, more for another time…)

The developer logs into the github repo and can see the new branch. He will now create a pull request (PR) (from the new branch to master) and follow the usual pull request process. Approver reviews the code and approves the PR. Now the transport is allowed to be released from the system. Note that we block the git merge from happening until later, as the transport is code reviewed but not yet released.

If the approver asks for a change in the code, then the changes should be added to the same transport, and when the developer releases the subtasks again, the branch and the PR is updated with the changes.

When the transport is released/imported to the target system, the new branch is then merged automatically in github to the master branch, thus ensuring github repo master branch and target ABAP system code base remains the same.

Benefits:

  • Leverage industry standards for code review (4 eye principle)
  • Every change in the transport is clearly visible in git, without the need to drill into each object in SAPGUI and compare versions
  • Ensure all code is reviewed before release which fulfils our audit requirement
  • Code review collaboration is captured nicely, allowing both developer and reviewer to comment at change level or even code level
  • No more surprises with unintended change reaching production
  • Many possibilities to integrate this with CI tool of choice.
  • Notifications are out of the box for github, meaning there is no need to chase someone for a code review.

 

Challenges:

  • You can’t just rely on github to review abap code
    • You still need SAPGUI/Eclipse
    • To fully understand the change, you might still need to go back to SAPGUI/Eclipse to fully understand the impact of the change.
  • Lots of legacy code without proper package strategy
    • We have a lot of legacy code. Our use of ABAP packages is not the best, resulting in logical code spread across multiple packages. The packages are big, and often code/objects are placed in the wrong packages.
    • abapGit assumes one abap package can only point to one github repository (rightly so)
    • Ideally you want all of the objects in your transport to belong to one package only but it is very hard to enforce. Failure to do so, the “transport to branch” feature will ignore all the objects that are not in the repository.
    • To solve this problem, we have a top main parent package encapsulating all of our packages, and we sync that to github. The downside is that it created a massive package containing lots of objects.
  • Enhancing abapGit code is risky
    • Our changes could be overwritten easily, or contradict/incompatible with the ongoing changes in abapGit open source code after we update our abapGit code from the internet.
  • Not all ABAP objects are supported by abapGit
    • For objects that are not supported by abapGit, we still need to capture a review has taken place.
  • Slowness
    • As explained above our repository is massive, over 50,000+ objects, and each time we open AbapGit takes a long time as it needs to serialize everything and compare with github for changes.
  • Protect this repository from abapGit
    • We want to allow access to the abapGit report, but not allow anyone to temper with the settings for the code review repo.
  • When doing a code review it is not easy to know about the transport dependencies

abapGit modifications:

We had to make a number of changes to abapGit for this to happen. We have not yet contributed the changes back to the wider community as we are still testing if this works.

  • abapGit will not let you sync a package if the parent or child package is already linked to the github repository. We had to relax this check.
  • We had to develop some code to automatically create a branch with the changes in the transport
  • We had to develop some code to check an approved PR exist before transport can be released
  • We had to develop some code to merge an approved PR post transport release

We also had to make these changes in a way that allows us to use abapGit for code review, whilst also allowing developers to use abapGit as it is intended.

Contributing back to the abapGit open source project

We are happy to contribute back to the open source community, but we first want to get some feedback on what others think about what we are doing. It is a bit different to what abapGit was intended for. Also what we’ve changed may not be agreed by others as like I said, abapGit was designed with a different purpose in mind.

 

Finally…

Well, hope you find this interesting, and do reach out if you need more information. I deliberately left out a lot of technical information to keep this blog simple.

13 Comments
You must be Logged on to comment or reply to a post.
  • I knew I read something like this before.   So click on the link in the paragraph.  It is an “easier” way to build this out.  Or not to really build it out but use GITHUB for code reviews.   Reading this new blog “just” automates it.

    “JUST” meaning adding a lot of work to automate the process. I would love to see it in open source.  For a large group, I can see this being a huge time saver.

    I’m wondering about smaller shops – like mine.  I think it is a bit daunting to even think about even putting this together.  I wonder about the time vs. advantage that type of thing.    Anyway would still love to see it.  If it isn’t a terrible challenge to do, I think I would even try to install it and do it here.

    Very cool idea!

    Michelle

    Edited comment after response below.  

    • Hi Michelle,

      Yes, there was a link to Eduardo’s blog in my first paragraph. It was a little bit of work to get going at first. We are in trial at the moment and will come back later to quantify the benefits and will discuss internally around the opportunity to open source this.

      Glad you liked it.

      Cheers

      Michael

       

  • Nice blog 🙂

    Enhancing abapGit code is risky

    IMHO it’s not risky to enhance abapGit. You can create a branch in your own abapGit fork an then rebase every now and then onto the updated master branch. That’s how we do it in our shop. Of course then you have to deal/resolve the conflicts. But I wouldn’t call that risky but the way to go.

    • Yeah, my corporate fork of abapGit carries 6 unpublished patches. The only thing I need to do is to regularly run “git pull -r” to rebase my patches with the most recent abapGit master.

      • Hi Christian/Guenter,

        abapGit is an active open source project. We’ve had scenarios where we changed some code, and a few months later after doing a “git pull”, either the class or the method we modified disappeared. Yes, we managed to recover our changes and find another place for it, but the maintenance requires a bit of effort. Hence I described as “risky” as after an update, we don’t have a lot of faith whether our solution will still work. We have yet to put some safeguarding in place. WIP.

        Cheers

         

         

  • You don’t need to immediately open PRs for abapGit but you can maintain your own public fork of abapGit, so the community can have a look at your changes, approach and technical solution and even run it in their landscape (I am really interested).

    BTW the benefit of git is not only the version of code but documentation of commit message. How do you enable developers releasing a transport task to describe the changes?

  • Sounds great. Maybe your topic should be more Something Like “How we do ABAP code review using github”. Cause you are not using abapgit for review 🙂

    My thoughts: If a developer releases his task you are creating a branch. But at this time its already activated in Dev system. In my opinion the Workflow should be: releasing Task, push changes to dev branch and creating a new pr to the branch of the next system (maybe qs) only containing the changes from the transport. With that you have a develop branch fitting exactly to your development system.

  • Thanks, nice blog, thanks for sharing

    abapGit is MIT licensed, so everyone can make forks if they like. But for forks both the community and the forking party typically looses, so I recommend not making forks. There are multiple ways to handle this, but all starts with an issue on github. abapGit also offers a set of user-exits for customer specific code https://docs.abapgit.org/ref-exits.html

    Also see https://github.com/abapGit/ci_tools which is meant as a set of tools for setting up scenarios like this, offering API calls to git hosts, but its work in progress

    https://github.com/abapGit/background_modes uses some of the same idea as above, offering additional background modes to make one branch per transport, again work in progress

  • Hi, Michael, great blog!

    I liked your twists on the code review approach. I am particularly curious about the ATC integration that you wrote. As the other mentioned, it would be nice if you could share the code, so we can take a look ;).