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:
- Our guidelines are available online via an easy to access website
- New developers should receive a two-page PDF-document via email with instructions where to find and how to access them. The 2nd page contains some important Do's and Don't Do's with links to the relevant sections in the overall guidelines
- The guidelines can also be accessed via SE80/SE84 and if they don't get displayed there yet, developers get some information about how to have them show up (plus a link of where to find them online)
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
Thanks to all who provided answers, comments, feedback and suggestions in this lively thread!
I just now picked Mike Pokraka's response as the "best answer" as his suggestion will be what I'm going to implement. All the other feedback and discussions in this thread also provided lots of food for thought and I have a hunch that there's ample material for a blog post or two just waiting to be written to summarise some of the content.
If it's okay, I'll not close the discussion so that it stays open for any additional thoughts and suggestions.
Thanks again & cheers
Baerbel
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.
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
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
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 )%
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
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...
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.
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.
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
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.
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
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:
- Readable
- Well modularised
- Maintainable
- Robust
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).
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.
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.
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:
- We only have a small inhouse development team working during office hours in Germany
- Development happens around the globe - in some countries with small and fairly stable teams, in others on an ad hoc basis with changing vendors, contractors and developers. As soon as you have some good rapport with a specific developer s/he often gets replaced by somebody else and you have to start from scratch.
- Sometimes whole projects get done by contractors and we just provide the guidelines to the team at the start (if feasible accompanied by a virtual session/workshop to clue them in) and then do spot checks on the delivered code throughout the project
- 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.
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
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.
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.
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.
You said
- 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.
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.
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
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.
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.
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.
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):
- Have a daily system message with some part of the guidelines.
- Have a UE\enhancement\Badi added to the SE38\80 that blocks the use of it unless they check mark the read of the guidelines.
- When opening a user for a developer have it in place that before receiving the user and password they have to confirm the guidelines or pass a quiz on them with some score threshold.
- Have a random programmer checked for those guidelines and email conclusions to the entire team (do that in a way you don't single out someone, did I say "ego canceling" already :) ).
- Send the programmers (unlike my first point which sends to all the users) a message (it could be done based on this - didn't test it myself).
- Periodic emailing.
- Single out your weakest link / most important project and focus your code review resources on it.
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 :)
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
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.
Reminds me of a code jam we did before TechEd a few years back...
It completely failed when we went to demo it, but... The answer you seek is gamification!��
Badges will surely help here... You can get badges for completing a review, finding different types of errors, responding to review comments... Ah the possibilities are endless!
Hook it in to the branch merge (pull request in GitHub) functionality of ABAP git, and we could have an ABAP development environment that might even be fun!
Unfortunately seems to be the sort of thing that only gets addressed in demojams.
Good luck!
Chris
If it goes like SCN gamification then soon the shifty consultants will start a "badge ring" where one would add an error on purpose and another would "find" it. :)
You could do the scoring according to least number of issues found. Of course, you'd have to carefully audit.
When I first encountered peer review, the programmers were put in pairs and we had to review each other's code. With my partner it got very competive and overtime we had maybe a couple of findings for each program. One time I had to review someone else's code - something like 60 findings. :-o
Hello everyone,
I see the challenges Bärbel mentioned as well in my organisation.
One of the key points is that you HAVE the funding of the program lead to "force" these rules an regulations, but here is where it comes short from time to time. People / Consultants / Contracors state: "this will have impact on the time line". Same discussion as with #unitTests. Yes, in the beginning ... But often, this is not accepted by management. ( with short term look )
Please: @Management: Let those geeks do their work an care for quality, similarity, and code perfection! it will pay out over the runtime of the system.
My Devices to overcome the mentioned hurdles are:
- Print some exemplars of you dev. rules and hand them out to every new developer.
- try to talk to them and ask them about their opinion to those rules and if they have advice, ( after they read them )
- use ATC & ABAP Open checks on a daily basis an talk about the result in dev. team meetings
- review your guidelines: not to overfloat them and rewrite an ABAP Book.
- offer information / training even to externals consultants how to use ATC to simplify their everyday work.
- try not to have so called "developing consultants" ( Consultants who learned to type ABAP, but do not have clue about Software development ) ( Goog key question is: What would you use an interface for? )
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.
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
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.
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?
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
First of all, very interesting point and discussion, thanks for starting it. We've had the same questions over the last years and were facing the same problem of how to enforce this and how to have enough (internal) capacity for code reviews, especially in larger projects. And we've definitely not solved the issue, especially the process one, however one thing that already helps to a certain extent is the following technical piece:
- We've started putting some of our guidelines (including naming conventions) into a Code Inspector Check Variant. We've developed some custom checks as well for things that are not available in SAP standard. I would like to mention this excellent resource on this occasion: Git repo with ABAP Open Checks.
- Every developer is encouraged to use this variant from the start of development and not only at the very end.
- The Check Variant is configured as mandatory on release of transport and any prio 1 or 2 finding will block the release. While externals are not allowed to release transports themselves they are still responsible for fixing their code until it can be released from DEV, so it will come back to them.
- Developers are adapting quickly and are making sure that their transports can be released and are not blocked for some days until they are back in the office and have the business area waiting in their door because testing could not be started in e.g. the QAS system.
- Make sure to communicate early and transparent with all developers. :)
So far our experience with this is quite positive and we will definitely continue on this route.
Cheers,
Alex
Thanks for your feedback, Alex!
Here is what we already have in place:
- Our own default check-variant for SCI/ATC which includes recommended checks for HANA-DB plus most of the ones in SAP's default variant - and any others I consider worthwhile to have
- For now, errors 1 & 2 are only flagged during transport release but don't prevent it. We'll wait with that until we have a central ATC-system set up which will allow working with a baseline, include S/4HANA checks and doing the checks during task-release. Developers have already been given a heads-up that this will happen down the line
- At a guess - and if you asked our developers! - I'm leaning towards communicating too much with them instead of too little!
Exhibit 1 - A news box next to the ToC on the homepage of our guidelines:
Exhibit 2 - A blog within the guidelines website to announce updates:
As not everybody subscribes to the blog posts I also send out links to them as needed, roughly once per quarter.
Bottom line: there's not much chance for an excuse like "I didn't know you had guidelines" or "I didn't find them" :-)
While automated checks are invaluable, there's no substitute for eyeball checks.
I've been developing in ABAP since 1997 - from junior developer to running multinational projects involving hundreds of developers. "There is not enough resource" or "we haven't the capacity" is absolutely not a valid reason for not carrying out peer code review. It should be part and parcel of the process. If you have enough resource to write a program, you have enough resource for one developer to review another's.
If you don't have peer review in place, your project will suffer in terms of quality and ultimately cost more and take longer.
Matthew, how would you organise code/peer review if you have to deal with almost constantly changing contracted developers? Your suggestion of pairing up developers seems to be geared towards a situation where you have a fairly stable developer base where long term connections and interactions can be established. Is there another means in a situation like that than doing them centralised which then more likely than not creates a bottleneck at least every now and then?
Is the only real option to have some kind of monetary penalties fixed in the contract if guidelines are obviously ignored (which I don't have a means to enforce but perhaps could suggest to those who have it)?
Personally, I keep wondering why contractors have such a hard time accepting guidelines even though it should be a no brainer of who (their customer) has the right to tell them (the vendor) of how things should be done.
Barbel,
Make them "A C C O U N T A B L E" for every development they handle and every bug that arises due to poor coding or lack of code review process.
One more thing is, may be it is a too taboo for a topic but still I try to put it with good intentions and not blaming anyone.
1.Timelines.
2.Sometimes Vendors will be forced to accept stringent timelines by competing against other Vendors ( monetary & Timelines ) going by the quote "Survival of the fittest" :-). People agree to stringent timelines at the cost of Quality.This is where Quality starts committing suicide gradually.
One gentleman said "There is always time to do a right thing in a right way" referring us not to hurry/rush and to plan wisely with ample time to accomodate everything to deliver a quality code.
K.Kiran.
You could get suppliers to review each other’s work, that will ensure thorough reviews!
But ultimately this sounds more like a governance failing in your organization if you don’t have the means to monitor or to hold people accountable. Trying to fix it with a technical solution just bypasses the problem temporarily.
You must have some kind of QA process in your own organization, even if it’s a spot check of random transport requests. Introduce penalties for subpar failure rates. Getting people to rework is not enough, I’ve had enough experience with cheap outsourcing to know that there are people out there happy enough to rework the same piece several times in order to have an easy life.
I echo with Mike....
"Trying to fix it with a technical solution just bypasses the problem temporarily."
"there are people out there happy enough to rework the same piece several times in order to have an easy life."
Well Said......
K.Kiran.
How often does your base off developers change? How often does one quit and another come join? If the volatility is too high, I wouldn't worry about reviews - there's no point because your code base is going to be low quality regardless.
You don't have to pair developers for review - that's an ideal. In each location/area/virtual group, I'd appoint one developer to be in charge of making sure reviews get done. When code is ready for review, he/she assigns someone to review it within their group. A review of a 5 day development typically takes less than an hour, so there really shouldn't be any bottlenecks. However, it's not right to think of program review as an add-on, where you end up with a bunch of finished programs that can't go to test because they've not been reviewed. If a program hasn't been reviewed, it isn't finished. It's as unfinished as if the program is syntactically incorrect, or doesn't implement a specific functionality.
Ultimately, if you haven't got control over your developers then it's an organisational issue. Why is there such high turnover? If you don't get a say on who works on your project, then you've been given responsibility without power - not a good place to be.
"Personally, I keep wondering why contractors have such a hard time accepting guidelines even though it should be a no brainer of who (their customer) has the right to tell them (the vendor) of how things should be done."
This is an industry-wide issue and I could write the whole book about this. It's not like they have hard time accepting. It's most likely your pesky guidelines are preventing the consultants from copy-pasting the same code they've been copy-pasting since 1999 or copy-pasting from SCN/other websites without thinking. Let's say they don't use OOP but now your guidelines say to use it. Oh, snap! Now the consultants are in mild panic. They can't admit to their managers that they don't know stuff, their own company doesn't train them (because otherwise they won't be cheap) and they can't pad enough billable hours to use for learning. So they go to their manager and start crying: "ah, I deliver the code that works just fine according to the specification but this customer lady is being really difficult and neat-picking on my code, now I'm afraid we can't make the deadline we promised". Consultant manager only hears the last part and starts calling your manager: "Hey, what's happening, our devs have delivered this great code, but now your team is causing a delay". Your manager does not want any delays either, so they start calling you: "hey, what's happening, are those guidelines really that important?" Basically, the guidelines is the absolute last things that everyone in this story, except you (or your team), cares about.
How to solve this - if I knew, I wouldn't be sitting here. You definitely need full support of the management (which can be quite difficult to get, sadly). And the guideline check needs to be as automated as possible to remove the possible blame from you or your team. E.g. like with my kid: if I tell him that he needs to go to bed in 10 min. and then after 10 min. tell him it's time then I'm a mean mom. But if I tell him that in 10 min. the timer will go off and he needs to go to bed then he can't really argue with the timer, can he? :)
Best of luck with this!
Thanks for your perspective, Jelena! It sounds fairly familiar ....
Thing is, that it's often readily apparent that the developers didn't even bother to read a one-pager with important do's and don't do's. Granted, we do have some rules which may raise some eyebrows (see below for an example) but these rules are spelled out right up front and apply to everybody. We have to live with them and so do external consultants/developers.
An examples of a "special" rule:
- Developers are not allowed to create their own transactions but need to request them from our basis team. The tcodes need to follow specific naming conventions related to general authorisations so SE93 has been locked. There's however an "exploitable" loophole for this restriction as tcodes can still be created via a right-click in SE80 (I'm not sure if there's a means to plug that?). So, whenever we see a new transaction not following the naming rules we know that the guidelines haven't been read at all.
Cheers
Baerbel
From my experience, it's more effective to have a short meeting/call to go over the guidelines. We had much less issues afterwards when we had a chance to do that with the consultants. But there is not always time for that, unfortunately. Based on the comments here, I feel that the constant rotation of the consultants is the real challenge in your organization. Perhaps it's something that you could bring up with the management?
No matter how well the guidelines are written or what steps you take to communicate them, there will always be higher failure rate when the staff rotates. It's very challenging and ultimately ineffective way to work.
Some cases though just fall under "you can't fix stupid", I'm afraid. E.g. one time a consultant created a transaction when no one even asked for it. It was a report that we'd run in a background job, so there was no need for a transaction code. I don't recall the exact explanation but I believe it was something like their past clients always wanted a transaction code or some such nonsense. So yeah...
Yes, a meeting is what we try to do if external consultants/developers are brought in for longer running projects. Thing is, that even when we provide the guidelines ahead of the meeting and specifically ask them to at least take a look at them and come with questions, more often than not, they come completely unprepared into the meeting. And even after such a meeting we happen on things they should have known better had they actually bothered to read the guidelines afterwards. To say that this is frustrating would be an understatement!