2018 Feb 06 6:40 AM
Hi Folks,
as you can tell, my question is more of a process than technical nature and I'm basically looking for some ideas and how-to's.
Here is what we already have in place to make our development guidelines available:
Where this often breaks down, is that the PDF-doc doesn't get send out in the first place and there's also no formal process to have the developer aknowledge that s/he received, read and understood the guidelines and plans to adhere to them.
Have any of you implemented some kind of process within SAP to streamline and document this? I'm for example wondering if it could work to send the PDF-document via the SAP Business Workplace as an express document to new developers, and have them aknowledge that they received and read them via response message. They'd then only get their developer keys after we received this response. Are there perhaps even some standard workflows for something like this?
Do you have other suggestions based on what is already done elsewhere?
Thanks for any feedback you have!
Cheers
Baerbel
2018 Feb 06 7:31 AM
This is so funny, because it's one of the simple examples I dreamt up for the Workflow book (Ch. 16.3.2 in the third edition if you have a copy somewhere in your office).
To summarize: Put a dummy task with all the information as the task text in a workflow, and use the task's 'confirm end of processing' flag to act as a confirmation. As part of your onboarding send it to them.
But when I read "we don't have a proper QA-process (yet)" I would really focus less on guidelines and more on code quality. Time spent on this type of thing is better spent, I've seen some truly awful code that confirms to guidelines, and on the flipside there is no one size fits all when it comes to guidelines. Use code inspector and start adding the worst transgressions to your custom checks.
2018 Feb 06 6:48 AM
Barbel,
I am answering your question assuming your emphasis is more on how to make the developer adhere to Dev guidelines.
Then,the only way is
1. Strictly enforcing code review and then let the developers take the corrective action on the review comments.
2. Only after Step 1 is done, approve the changes to be moved to QA.
K.Kiran.
2018 Feb 06 7:05 AM
Kiran,
thanks for your answer and I'm well aware that we should do this but we don't have a proper QA-process (yet) - at most we manage to do some spot checks for larger projects. Adherence to the guidelines basically hinges on the developers' good will and I'd at least want to prompt them to access and read the guidelines as our spot checks tend to show that many developers didn't even bother to read them (and not just for what we see in the code, but also for not following some of our clearly outlined specific processes and restrictions).
As a first line of defense, I want to take away excuses like "I didn't know the guidelines existed" or "I couldn't find them". To tell you the truth, I even contemplated to bury an "Easter egg" somewhere in the guidelines and then do a short quiz to see if they actually read them by adding a question which could only be answered correctly when that "Easter egg" was spotted. Developer keys would only be handed out upon successful completion of the quiz. A bit over the top, I know but perhaps still an option.
Does this put my question into perspective?
Cheers
Baerbel
2018 Feb 06 7:12 AM
Got it 🙂
"Adherence to the guidelines basically hinges on the developers' good will"
E X A C T L Y
Other way is link it to Money.
For every NCA ( Non Compliance ), penalise them 🙂
Cost saving and over a period of time, People will fall in line.
K.Kiran.
2018 Feb 06 7:31 AM
This is so funny, because it's one of the simple examples I dreamt up for the Workflow book (Ch. 16.3.2 in the third edition if you have a copy somewhere in your office).
To summarize: Put a dummy task with all the information as the task text in a workflow, and use the task's 'confirm end of processing' flag to act as a confirmation. As part of your onboarding send it to them.
But when I read "we don't have a proper QA-process (yet)" I would really focus less on guidelines and more on code quality. Time spent on this type of thing is better spent, I've seen some truly awful code that confirms to guidelines, and on the flipside there is no one size fits all when it comes to guidelines. Use code inspector and start adding the worst transgressions to your custom checks.
2018 Feb 06 2:51 PM
Thanks for the pointer to your example, Mike!
My colleagues who tackle workflows have the book and I briefly chatted with one of them about your example. This may well turn out to be a workable option for what I have in mind, so I'll make a suggestion to have it implemented.
Cheers
Baerbel
2018 Mar 02 1:39 PM
Hi Mike!
I finally got around to prototyping your suggestion in a sandbox system and already have the workflow and workitem defined. I'm hitting a little snag though when I try to define the "Latest End" for the workitem via the special notation for the expression shown in your book chapter:
I've compared this with what's shown in the book and I (as well as a colleague) cannot see any difference. Is it possible that something changed since the book was published in 2015 and our current SAP version which is NW 750 with EHP8 which requires another format for the expression?
As I know next to nothing about workflows and am just working from the description in the book, it's possible that I'm missing something, but the workflow and workitem generated okay.
Thanks & Cheers
Baerbel
2018 Mar 02 1:58 PM
Hi Bärbel,
You had me worried so I tested it on my 7.5 SP6 system and it works. After pressing enter it shows the expression to the right of the field. Two things to verify:
Is the class active and free of syntax errors? Does it have a RETURNING parameter of a date type?
The code I just tested:
CLASS zcl_date DEFINITION PUBLIC.
PUBLIC SECTION.
CLASS-METHODS get_date IMPORTING i TYPE i
RETURNING VALUE(result) TYPE d.
ENDCLASS.
CLASS zcl_date IMPLEMENTATION.
METHOD get_date.
result = sy-datum + i.
ENDMETHOD.
ENDCLASS.
And the expression entered in the field:
%zcl_date.get_date( 3 )%
2018 Mar 02 3:01 PM
Hi Mike,
thanks for your quick response! This is what I have in the class and method based on the description in the book
Class:
class ZCL_WF_UTIL definition
public
final
create public .
public section.
interfaces BI_OBJECT .
interfaces BI_PERSISTENT .
interfaces IF_WORKFLOW .
class-data GUIDELINES_URL type STRING .
class-methods NOTIFY_DEVELOPER .
class-methods CLASS_CONSTRUCTOR .
class-methods GET_FC_DATE
importing
!I_OFFSET type SWD_TIME_O
returning
value(R_DATE) type SWF_DLDATE .
protected section.
private section.
ENDCLASS.
Method:
* <SIGNATURE>---------------------------------------------------------------------------------------+
* | Static Public Method ZCL_WF_UTIL=>GET_FC_DATE
* +-------------------------------------------------------------------------------------------------+
* | [--->] I_OFFSET TYPE SWD_TIME_O
* | [<-()] R_DATE TYPE SWF_DLDATE
* +--------------------------------------------------------------------------------------</SIGNATURE>
method GET_FC_DATE.
"Calculate n workdays from today based on factory calendar
CALL FUNCTION 'END_TIME_DETERMINE'
EXPORTING
DURATION = i_offset
UNIT = 'TAG'
FACTORY_CALENDAR = '99'
IMPORTING
END_DATE = r_date
EXCEPTIONS
FACTORY_CALENDAR_NOT_FOUND = 1
DATE_OUT_OF_CALENDAR_RANGE = 2
DATE_NOT_VALID = 3
UNIT_CONVERSION_ERROR = 4
SI_UNIT_MISSING = 5
PARAMETERS_NO_VALID = 6
OTHERS = 7.
IF sy-subrc NE 0.
"Return default date without calendar calculation
r_date = sy-datum + i_offset.
ENDIF.
endmethod.
I don't really see where this is different to what you have apart of course the various names and that I use the function module. I just tested the method directly and it works as expected.
And now for the really weird part! I just added the expression again and guess what? No syntax error!
Due to some other issues, I had to logoff from and logon to the system again so perhaps that cleaned up something or other in the buffer.
Cheers
Bärbel
2018 Mar 02 3:08 PM
Great!
It was probably the first case, my guess is the class was incomplete / had an error when you started the WF builder.
If you then modify and activate the class in another session, the WF builder will continue to use the loaded class until you exit the transaction and start it again. I still get caught out by this one...
2018 Mar 05 1:25 PM
Hi Mike,
me again (and I hope you don't mind)!
Technically it seems to be working now that I set up things as outlined in chapters 16.3. and 16.4 in the book. I however seem to be missing some information - and it's possible that I just overlooked it as I don't quite know what I'm doing as I'm not at all familiar with either Workflow or ABAP OO and am just working from what is in the book (which is why I do all of this in a sandbox system):
What do I need to implement and where (in the Workflow-definition and/or one of the classes?) to actually send the pop-up to somebody else but myself? I was expecting to somewhere see the definition of an input parameter to feed a user-ID into the Workflow, but - even reading back through the chapter - I can't find it.
What I eventually plan to do is write a small ABAP-program to trigger the workflow for one or more user-IDs which have then already been verified to be valid and for developers.
Please let me know if you need any screenshots and I'll put a document together.
2018 Mar 06 7:08 AM
Hi Bärbel ,
No problem. The agent field on the workflow step is where you define the recipient. The quick and dirty is to define a container element with the userid and start the workflow directly with FM SAP_WAPI_START_WORKFLOW.
However the 'right' way is to create a user object and define that as an importing parameter for your WF, and use an event to trigger the workflow. This way WF is decoupled from your application. Most of this is part of the same chapter - although you definitely don't need to go the ABAP OO Events route, use the simple variant.
You don't even need a report, you could also raise this event from anywhere, in your user creation process for example.
But I think it's moving off the original topic, if we're going deeper into this I would suggest posting a new question under the workflow tag.
2018 Mar 06 9:54 AM
Thanks again, Mike!
It seems to be working and I'll let a colleague who knows more about workflows than I do take a look before I re-create this in the actual development system.
Once I have it working there, I may summarise things in a blog post (unless that gets too close to what's described in the WF-book?).
As this may take a while, I'll set this Q&A to answered.
Cheers
Baerbel
2018 Mar 06 8:25 PM
Hi Bärbel,
That’s great news. By all means go ahead and blog, publicity is always good 🙂
(By the way I don’t make any money from the book, all authors’ proceeds go to charity.)
Regards,
Mike
PS, it’s a shame the new platform doesn’t have the ‘helpful answer’ feature as this question in particular had so many good contributions! I don’t think an upvote counts quite as much and giving full credit to just one answer doesn’t really do full justice.
2018 Mar 07 6:23 AM
Hi Mike,
cool that the authors' proceeds go to charity!
And a heartfelt "Yes" to your PS! I would have loved to pick several helpful answers but as that is not available I went with a "thank you all" comment to my OP.
Cheers
Baerbel
2018 Feb 06 8:05 AM
I've worked in many places. In some, developers have a good attitude and you can trust them to read and adhere to the standards. In other places, the only way to get them to adhere is to stand over them with a hand gun (I think this is called "pair" or "extreme" programming). Seriously, if you've got externals who won't do what they're told - sack 'em. For internals, you just have to do whatever the social acceptable form of beating is in your organisation. I've no time for people who won't adhere to a few simple rules - either they're not to be trusted, or they're incompetent.
I have a good attitude, so I always make sure to read the standards and adhere to them - not matter how daft I might find some of the them. (e.g. one client who insists that all variables must be declared at the top of the code unit - well, that'll work well when 7.4 turns up...with inline declaration!)
Peer code review is a good approach if you don't have enough resource to have a separate QA section. It also means that you've got (supposed) experts doing the work. This does rely on attitude. I remember one project I was on, where it was done diligently, as the developers realised that they could learn off each other not just the standards, but good techniques. It became a game to find violations. In another project, the developers found that the regulatory authorities were happy with a signature that a review had been carried out - so they just signed off every development without looking at them. I was in charge of all the developers across the project, and did a few spot checks and discovered this - unfortunately, the higher management team didn't give a monkey's about actual quality, so they took no action.
I did a webinar for SCN 9 years ago on this subject, but the video appears to be no longer available. However, this blog is worth reading and you can see my comment there. https://blogs.sap.com/2015/01/27/code-review-success-factors/
An important factor in peer code review is that a junior reviews a senior and vice versa. That way everyone benefits and it spreads the load and programming knowledge and good practice.
If your standards don't contain too much that's irrelevant, that will help. Concise standards are far more likely to be adhered to than a fifty page document. Don't sweat the small stuff. I strongly recommend removing anything that says that, e.g. local variables must be prefixed with l_ . Rather follow Horst Kellers guidelines in the book "Official ABAP Programming Guidelines".
A few years ago, I got to define the standards for a client.
All code must be:
That was it. But that was with a small, close knit team. For a larger team, you do have to enlarge on what those terms means and give guidelines for achieving them.
For most clients, I have to sign I've read the guidelines and standards and will adhere to them. For some, everyone now and then I have to sign I've re-read them. For one, this is achieved and recorded through LSO. In their validated systems, there are ATC checks that must be passed. This works quite well, as it has spotted some programming issues that I'd missed. If you have some processes and restrictions that are an absolute must, put them into ATC and block release of transports that don't pass the checks.
But to my mind, for improving code quality and ensuring adherance to the rules, nothing beats a peer code review (with spot checks to spot sign offs without checking).
2018 Feb 06 8:15 AM
Matt,
"An important factor in peer code review is that a junior reviews a senior and vice versa. That way everyone benefits and it spreads the load and programming knowledge and good practice."
I liked your detailed reply.
Thanks for sharing your thoughts.
K.Kiran.
2018 Feb 06 11:46 AM
Nice blog. At least it's worthy of a copy/paste into a blog.
"Concise standards are far more likely to be adhered to than a fifty page document.", that's pretty much the point I was trying to get at when I wrote about focusing on code quality.
My ideal approach is "Follow the SAP coding guidelines in the online help" (pretty much all of the book you mention is in there), and then any additional guidelines not covered in the help or where company practice differs.
At the extreme end, I've seen guidelines obviously written by someone who has never actually implemented the guideline themselves. e.g. "Exception classes must include the module name as the first prefix, Z<module>CX (so ZMMCX_ for an MM exception class and so on), but SAP will throw an error if you actually try to do that.
2018 Feb 06 1:27 PM
Thanks, Matthew - lots of food for thought in your response!
In our case, "the only way to get them to adhere is to stand over them with a hand gun..." is just a wee bit difficult to do though, even in its intended metaphorical interpration. Here are some reasons why:
Having said that, I'd already be happy for now if I had a means to get developers to aknowledge the guidelines without having to send them individual emails and having to keep track of the responses.
Cheers
Baerbel
2018 Feb 06 1:41 PM
Baerbel,
"Code reviews cannot always happen before the code leaves development because they cannot hold back dev-work for a project and a lot of stuff unfortunately cannot be properly tested in dev due to missing data/interface-infrastructure but only in QA."
You can ask the Basis team to refresh the DEV server with PRD data at regular intervals.
K.Kiran.
2018 Feb 06 1:45 PM
Kiran - refreshing DEV from PROD is not really an option in our system landscape and setup. We don't even do this very often for QA with all the issues it entails to not have even remotely current data in QA.
2018 Feb 06 4:17 PM
You said
Define "finished" to include "has undergone peer code review".
Why? Peer review is part of the coding process. The program isn't finished until someone else has run their eyeballs over the source code checking for errors, adherence to standards and that it is a of good quality.
You do this as early in the process as possible, certainly before testing, as the earlier you pick up issues, the cheaper and easier it is to fix.
I can guarantee that if you have a good peer review process, you will save project time (and reduce costs). You will also improve the overall quality of your code base. Good quality code is easier and quicker to maintain and enhance, and therefore (guess what?) cheaper. This is development management 101.
OK. Now let's suppose that developers really do need to test in QA before they can finish their programs. In that case, you allow them to create transport of copies of their open requests that only can go to QA. Allow these ToC's to be imported regularly. Now your devs can test their code incrementally on QA. When they're happy with their unit test, they hand their source code over for peer review, then when all is well, they release their original transport .- now the testers can get their sticky mitts on the applications.
Summary: there's few valid reasons to not do peer reviews. I'd go as far as saying that in a project there is no valid reason for not doing peer reviews. If you work in Pharma, the FDA require code reviews, and I'm sure other regulatory bodies do as well. You might have to modify your architecture and / or processes and policies to achieve it, but it can be done and it is worth it.
2018 Feb 06 4:19 PM
Refresh DEV from QA? Problem solved! 😉
2018 Feb 07 6:18 AM
Sounds easy in theory but isn't in our case as QA is a clone from PROD and therefore way too much data for the relatively small DEV environments we have.
2018 Feb 07 6:45 AM
I agree with what you write, Matthew - I don't need to be convinced that proper code/peer review is necessary and in the end a time & money saver. What is lacking is a foolproof business case which will convince every stakeholder in a given project that this is actually the case. As with other things like documentation, QA tends to be on the back burner of many people's minds who only see the go-live date which needs to be met at all costs (somewhat like in this Commitstrip comic).
In addition, our basis team so far doesn't allow transports of copies so that's not an option at the moment. I'm not sure if I'll be able to talk them into allowing this as I just don't know enough about the advantages and the disadvantages of transport of copies to really argue with their objections. Is there a short description somewhere I could use to at least get a discussion with them started?
Cheers
Baerbel
2018 Feb 07 8:41 AM
There is no foolproof business case. A cynic might say that's because fools are far too inventive... I've managed to implement it through saying diplomatically that I'm paid to be the development process expert. My x years experience has shown me that this works (quoting the research, and giving good explanations why there is benefit), therefore I'm doing it and the project will benefit. One time I even said that if there was no perceived benefit, then I would quit once the project was complete. Asking for a business case to me is like asking for a business case to use one method or two in a class...
I understand that if you're a pharma company and the system is validated, code review is an FDA requirement. That's not a bad business case. 😉
I remembered a blog showing the Transport of Copies process, and while looking for it I found this, which suggests that the ToC process is even a SAP supplied option in Solman managed landscapes. I think this is the blog I was thinking of. I know that the process is in place, and automated, in a number of very large companies.
The basic idea is that you generate a ToC from the workbench transport. The ToC is then released. It's easy to write a program that does this for you, given a workbench transport number as input. The QA system is set up to import automatically every 15 minutes (for example) - 2-4 hours is also not uncommon. The transport landscape is defined with a hard rule of "No ToCs in production".
Now the developer can write her first draft, do some minor testing, and then ToC to QA and test. If there are errors, she can correct and the ToC again, until there are no errors. At my current site, the functional guys also test - when they're happy there are no errors, then the original workbench transport is released, and that is the one that goes to production,.
A side benefit is that each ToC creates a new version in the version database. If you ever have to revert mid development, this is very useful - like when a fix breaks another part of the same application.
2018 Feb 07 8:44 AM
You're gonna need a bigger boat... er... DEV box. 😄
2018 Feb 07 5:02 PM
I think there was a note released just recently that allows to exclude TOC from the version history. That was kind of an annoyance for us before. It's interesting that you see it as an advantage.
2018 Feb 19 2:53 PM
Why is it annoyance?
If you use some visible identification of ToC (for example I started to add "(TOC)" text at the end of transport description, automaticaly by custom ToC creation tool), then you can clearly see development flow in versions.
2018 Feb 19 4:31 PM
The version screen in SE38 only shows the transport number, it doesn't show the description. At least that's how it looks in EHP6 ABAP 7.31. We also always put TOC in the description but it didn't help in the version control.
2018 Feb 20 7:02 AM
There is no F6 - "Request text on/off" in version screen?
2018 Feb 20 3:12 PM
LOL. I never noticed it. Learned something today, thanks! 🙂
Can't say it's super convenient though because the description replaces date/user columns (even though there are tons of space on my wide screen monitor). Still, it'd be much preferable to just exclude TOC. In some cases, I had 10 if not more TOCs created for the same program because there was just a lot of trial and error involved that was not possible without real data.
2019 Jan 09 3:11 PM
> There is no F6 - "Request text on/off" in version screen?
Oh, I've seen that button before, but never cared for it!
So I, too, learnd something new here, thanks tomas.buryanek !
(Also: with SAP_BASIS753 it's always there)
2018 Feb 06 10:28 AM
Another approach would be to use a BAdI upon Request / Release of Transport, with some kind of Checklist (ticking the Boxes for 'having done a Unit-test' and 'Read and Conformed to the Coding Guidelines' as an example) preventing Code from being "Released in the Wild" ...
By storing and reporting on these indicators (through ad hoc Peer Review?) you might have something 'in writing' as a Point of Discussion or Reprimanding the Developer ...
Just to be clear : we haven't implemented anything like this at my Company, but I have thought about something like this in the past. Inspiration was gathered through this Blog : link
So even though this is an "after the facts" -approach, it could atleast make the general Compliance with your Coding Guidelines and Culture visible?
2018 Feb 07 10:38 AM
Thanks for your feedback, Nic!
A while ago I "played" a bit with the BAdI you mention (ZCL_IM_CTS_REQUEST_CHECK), but for the transport creation to try and get developers to put some specific type of information into the title. These trials however never made it out of a sandbox system thus far. The aspect of running SCI during task- instead of transport-release will hopefully soon get tackled by setting up a central ATC-system with the latest version which will allow this option via configuration of the checks. Once we have this system up and running, we'll also start making use of preventing release of a task/transport with errors.
Cheers
Baerbel
2018 Feb 06 8:58 PM
Hi,
Let me start by saying that the last question you posted (and referenced here) is a remarkable example of how to use this platform, and it looks like you are keeping it up (maybe consider blogging as well, we could all benefit from it, I believe).
Now to the matter at hand, as I second matthew.billingham point of the importance of code review, I would also add that you need to put in place some "ego canceling" method (e.g. someone that code review is its only job and that every remark is being handled professionally and with reasoning behind it to justify it).
As for confirming your dev. guidelines, here are some practical ways to enforce it (some I saw in previous working places and some are a figment of my imagination, but, I think, doable):
This list is, of course, incomplete and is adding (sometimes even in a tangent) to other points that were already made.
As for adhering to the guidelines, apart from what was already been said I would add, choose the best and brightest people you can, to begin with, insist on good programmers and demand the ones you established rapport with - you are paying for it after all. Have a longer process of interviewing/allowing a contractor to work with you. This advice is a cliche by now for a reason 🙂
2018 Feb 07 6:58 AM
Thanks for your feedback, Iftah!
As you bring up blogging, I was going to ask if this ongoing discussion with my initial questions and the valuable feedback provided could/should perhaps be turned into a blog post to give it a more permanent home. Given your comment, I guess that the answer to that is "Yes"?
Cheers
Baerbel
2018 Feb 07 7:06 AM
2018 Feb 07 11:43 AM
This is one of the most informative discussions I've seen in a while, with a lot of relevant information all in one place. I definitively see value in having this up as a blog. Or maybe a blog about the final solution that links back here.
2018 Feb 07 1:17 PM
When I have some time, I'll deal with it.