Charter

THIS PAGE IS NO LONGER UPDATED.  See the latest gate page for updated documentation.

What is a CRT? The ON CRT (OS/Net Change Review Team) is chartered by the ON C-team to deal with the day-to-day business of evaluating the technical risk and merit of proposed changes. Its powers are thus derived from the C-team's and constrained by the SDF. It may not say no to a complete, well-formed change request that meets the requirements set out by the SDF, although it may ensure that adequate steps are taken to evaluate and if necessary mitigate risks attendant to it.

 The ON CRT is an example of the SDF “shrink-to-fit” model; the C-team deals with “projects”; the CRT deals with changes that are smaller in scope. One thing that the SDF does not cover is post-FCS change. The CRT does, however, review patch requests as well. Its ally in this is the ON business team, which works for the PAC and is the body to which CRT members may turn when they are asked to act on patch RTIs which do not adhere to the fatal defects criterion outlined below, or which they view as simply too risky to patch.

History

 The OS and Networking organizations have had the concept of a change review team for many years. It began early in the 5.0 effort, when it became clear that the rate and quality of change were out of control. The criteria we have asked engineers to meet have varied greatly over time, and the inter- action has gradually become more formalized. In the 5.1 days, changes needed the approval of either a manager or an engineer of staff rank or above. This finally evolved into a team of senior engineers whose membership varies over time, and a single lead engineer.

 When some alarming quality problems occurred in the 2.3 kernel jumbo patch, one of the many steps taken in an attempt to improve patch quality was to initiate CRT review of patch requests. It is this role which has evolved the most over the last few years, including the birth of and interaction with the ON business team. The CRT's role in the patch process is now well-defined: review the technical risk and merit of proposed changes.

 Today's CRT has no formal membership criteria. We look for engineers with a breadth of knowledge about the system as a whole, a specialization in one or more areas of OS or networking, a familiarity with our development processes, a deep appreciation for the risks and consequences of change, and the respect and trust of the engineering community. By convention, the tech leads of the release under development and the current update release are automatically members.

Goals

 The CRT has a single long-term goal: to put itself out of business. We believe that the ideal development environment is one in which each engineer has either the skills necessary to do his or her job, or the sense to find a peer with these skills who is willing and able to help. While this ideal world is under construction, we use these short-term goals:

risk assessment and bug prevention

We strive to make sure the CRT is composed of engineers who have enough experience to recognize whether a proposed change is ready for integration. Is it complete? Is it the appropriate thing to do? Is there a better way to accomplish the change? Should more testing be done? Is additional code review necessary?
 By doing this, we hope to lessen the risk to the system and to catch bugs before integration.

mentoring

Software development is an extremely complex task. Mistakes will always be made; some may be simple coding errors, while others may be caused by incomplete understanding of the system as a whole. CRT members should view the change review process as an opportunity to help less experienced or less careful engineers grow and improve.

ensure that we follow the SDF

Interactions with engineers

 Some time ago CRT members began being referred to as “advocates.” In what sense are we advocates? We serve as a guide for new engineers through the RTI process; we serve as advocates of good engineering practices to the engineering community; and, on occasion we serve as the advocate for a change to the ON business team.

 The primary interaction between us and engineering is by way of the RTI process. The tendency over time is for us to view the CRT as defenders of the source code from the invading infidels who would infest it with bugs. Instead, the RTI process should be viewed as an opportunity to help our fellow engineers.

 Mentoring is intended to be a substantial part of our responsibilities. It has been hypothesized that our development process has the undesired effect of lessening the personal responsibility that engineers should feel for their changes. We must make sure that engineers believe that they - not the CRT and not the gatekeepers - are responsible for ensuring the quality of the code they deliver.

 It is also important for engineers to appreciate the risk of making changes. We should help outfit them with the knowledge and tools necessary to understand the impact of the changes they make. They must also be made to believe in their inherent fallibility, and that bugs found before integration are successes, not failures.

 Code reviews are very important. We view the engineer making a change and the reviewer of the change as equally responsible for its quality. We should do what we can to ensure that code reviews and reviewers are high in quality. Toward that end, we expect you to be familiar with the skills of the people who review changes in the area(s) for which you receive RTIs. Do not hesitate to ask for additional reviewers, especially if a change spans more than one area. CRT members also serve as code reviewers of last resort; REs may always turn to the CRT when no other appropriate reviewer can be found.

 Testing is, of course, mandatory. It is the engineer's job to ensure that adequate testing has taken place, and our job to make sure he or she is aware of this and has done it. If you feel it is necessary, it is perfectly appropriate to require more testing be done. At the same time, you may want to explain why you are asking for it - view it as another component of mentoring.

Day-to-day business

Dealing with RTIs

 RTIs come in two flavors: “Marketing Release” (formerly known as “implementor” under the old rtitool system, which apply to the release currently under development), and “Patch” (known as “patch” under rtitool, which apply to releases that have already shipped). RTIs are dealt with via our web front end, WebRTI. Bugs or RFEs applicable to WebRTI are submitted via Bugster using the product/category webrti/webrti.

 When someone submits an RTI to you, you will receive notification via mail. RTIs, like bug reports, live forever. It is your responsibility to ensure that the information in the RTI and bug report is correct and sufficient. Like code, bug reports and RTIs are read many times for each time they are written. Much is said elsewhere about what constitutes a good bug report. A good RTI should be consistent with the bug report:

• Is this an API change? Is it so marked? If there is an API change, should there be a corresponding documentation change? If this is an API change to a shared library, are the mapfile versioning changes made correctly?
• Is an ARC case necessary? In the case of an API or ABI change, the answer is quite likely to be yes. If ARC case citations are in the RTI, are the cases approved? Are there any TCRs? Have they been addressed?
• Is there a documentation change? Does the bug report have the correct value for the “Fix Affects Documentation” field? Does the RTI indicate documentation impact?
• Why is this change being made? If you or the RE know enough to fill in the bug's Root Cause, be sure to do so. It helps the various quality efforts notice unhealthy trends.
• See the RTI nits page for more.

 The RTI should indicate that adequate code review and testing have been completed. You may, if you wish, review the code change yourself. If you don't know or don't trust the code reviewer, it is a very good idea to do so. If you find something wrong, you should tell both the RE and his or her reviewer (remember: we view the RE and the code reviewer as being equally responsible for the correctness of a proposed change).

 It is quite reasonable to ask that details on the testing be included in the RTI. Note that we care (of course) not simply about testing that indicates the change has been made correctly, but also that the resulting system is as stable and well-performing as it was before the change was made. Keep an eye out for changes that may have performance or scalability impact, and suggest appropriate tests to check for any degradation. If tests have been developed or updated in the course of making a change, be sure that they find their way to the appropriate repository.

 One of the primary ways we add value is that as a body we have a great deal more experience with our system than the average engineer. Ask yourself whether you know something about the proposed change that the RE does not:

• Are there packaging changes that are not being made (or not being made correctly)? Does this change need an upgrade script?
• Does this break an API or ABI?
• Does it break something that (incorrectly) relies on the format of some data structure? A good example of this is the knowledge that some of our graphics drivers have about the VM system.
• Is this an appropriate change for an RTI? Should it instead be a project, and be reviewed by the C-team for the current release?

 Remember that you are not alone: if you feel that another member would be a better evaluator for the RTI, feel free to forward the RTI. In a pinch, you may always forward the RTI to the current tech lead. If you have doubts about the correct course of action for an RTI, ask a fellow team member, or even the entire team. We are nothing if not opinionated.

 We strive for consistency between team members. This becomes more difficult as we get larger, and it is the primary reason that the CRT is kept small. If you have doubts about an RTI, asking opinions of the team as a whole is a very good way to help us achieve this consistency.

 From time to time it may be necessary to have a conversation with a troublesome RE's manager. If appropriate, do not hesitate to do so; the sooner the manager is made aware of a problem, the sooner he or she can begin to correct the problem.

 If you have questions that need to be answered before you can properly evaluate an RTI, you can handle it either inside or outside the RTI process. If you decide to do so inside the process, put the RTI on hold and tell the RE what must be done in order to get it off it held state. It is inevitable that on occasion the answers to one round of questions will raise even more questions, but you should try not to subject the RE to round after round of questions. If you know you will have more questions, tell the RE so his/her expectations are properly set.

 RTIs must be acted upon in a timely manner. Do not let them languish. If you are going on vacation, be sure you forward your RTI evaluation responsibilities to another member of the team (be sure you first ask the person you will be forwarding to).

Daily RTI Summaries

 The entire CRT, and several interested parties, receive daily summaries of RTI actions taken that day. You should peruse these summaries and take appropriate action if you see a situation that needs your involvement (an inappropriate change about to be made, for example). This should not be viewed as second-guessing your fellow team members. None of us is omniscient; we can all use help.

Dealing with patches and update releases

 Our current development and support model consists of two trains under active development, one of which has not yet shipped and one which represents the most recently shipped release (the update release). Additionally, there are patch areas for many previous releases. Each of the three types of environments can be the target of an RTI, and each has different RTI criteria.

 The release under development is covered above. For update releases, RTIs should go through the C-team and their CRT representative. For patches, we apply a single criterion to each request: we fix only fatal defects in a completed product. Each proposed patch change has the potential to negatively affect many customers if not done correctly, and for that reason we stress the need to accept only compatible, very well-tested changes.

 Furthermore, the CRT only evaluates the technical risk of such changes. If a patch RTI does not fit the fatal defect criterion, or you find that the decision is not a purely technical one, the CRT is not the appropriate body to make the decision.

 When the CRT was asked to begin reviewing post-FCS changes as well, its role was unclear. As time passed, it was recognized that quite a number of patch requests have a significant business component and that it was wise to separate the technical review of the proposed change from any possible business justification. The role of the CRT is to make risk decisions based solely on technical merit and technical data. The ON Business team works for the ON steering committee and is the body to which CRT advocates may turn when they are asked to act on patch RTIs which do not adhere to the fatal defects criterion or which they view as simply too risky to patch.

 While it is unusual to turn to the business team for an RTI decision, it should be viewed as the normal way of handling some RTIs and you should not hesitate to bring them in when the decision to be made is not purely a technical one.

 When presented with a patch RTI, one of several approaches may be appropriate:

get more information

in cases where there is an obvious and sufficient work-around, explanation is needed to understand why a patch should still be generated. Remember: we do not issue patches simply because a bug exists or a customer is somewhat inconvenienced. A bug with a work-around is not a fatal defect.

say no

this is the obvious choice for a patch request without obvious technical merit and with no business case (note that you are not being asked to judge the merits of a business case, simply recognize that one does not exist), or one with reasonable work-around(s). This would be the case for most RFEs. To say no, simply put an appropriate comment in the RTI and cancel it.

pass to ON-Business

this is the choice for a patch request which may not have technical merit, or which is a feature patch request, but for which a business case does exist, or one which is judged too risky to patch.

change it

patches should embody the minimally appropriate change. They are, therefore, not necessarily the same as the changes that should be made in the release under development. Watch out for patch requests that are overly broad. Do not hesitate to require a more constrained change.

hold it

if the change would be acceptable, but has not yet seen sufficient exposure in the current release. Except in unusual circumstances, a patch request must first soak (for at least 4 weeks) in the current release before it becomes eligible for patch approval and integration.

accept it

if you are convinced that both the patch request itself and the code change is appropriate, this is the right thing to do. It is not necessary that the RE integrate the change into the release under development before a patch RTI may be accepted. However, it is appropriate to require that he or she perform any steps you believe are necessary to mitigate the proposed change's risk.

 Before passing a patch request on to ON-Business, evaluate the problem and its proposed solution:

• if work-arounds have been missed by the RE, suggest them and be sure that either you or the RE find out whether any of them are sufficient for the customer (and if not, why not).
• is the change appropriate? What are its consequences? Is there an alternative which would solve the problem, but which is less risky?

 Having been satisfied that it is appropriate to pass this request on to ON-Business and that you now have sufficient data to answer the questions that will arise, bundle the patch RTI along with any recommendations you wish to make and send them to ON-Business, either by interacting with a business team member directly or by sending mail to <on-business@sfbay.sun.com>. In the meantime, be sure to put the RTI on hold with a comment that you are awaiting a decision from the business team.