RTI nits to avoid

Submitters

• Security issues
  • Sun has to walk a fine line with security fixes between customers on older releases who need patches and customers running the latest stuff for which the source is available. Sometimes this involves coordinating patch releases with 3rd party organizations and other vendors. Thus we handle each such issue individually; to do so, please coordinate with <scteam at sun dot com>.
• Mixed (closed and open) source integrations with Nevada build 97 and later:
  • You should have one RTI with all of the relevant output, as outlined below and in webRTI, for both gates.
  • You will need to do two hg push commands, one from each repo. Closed needs to be pushed first.
• SCCS Keywords
  • For Nevada builds 97 and later:
    • ident usage (whether #pragma, ident, or .ident) should be cleaned up incrementally. If you touch a file, you should remove these strings. The hg keywords (part of hg nits and hg pbchk) command should correctly complain about this. So look for clean hg keywords output.
    • Any other usage SHOULD have been cleaned up prior to the Mercurial transition, but it's possible that some was missed. For this, please remove the usage entirely. If that is an unacceptable answer, and you deem the usage to be an interface, the keywords may be replaced with their static expansion (or incremented therefrom), or the user may use hg id or hg log output for version information. The hg id command refers to the entire repository, while the hg log command refers to a file or set of files.
  • For Solaris 10 Updates, older sustaining releases:
    • SCCS keywords, where used, must not be expanded in your source code. wx nits will verify this for you.
    • SCCS keywords should not be used as an interface for checking versioning or be output in any way, shape or form. Instead leverage existing information like the list of installed packages and mdb \ commands (::showrev -v and ::objects -v).
• Testing
  • Developers are responsible for validating their changes prior to integration. This work should be described in the "Test Results" section of the RTI.
    
    Developers are also responsible for anticipating the need for ongoing regression testing of their changes.
    
    These considerations should be described in the "Comments" section of the RTI, and should include:
    • Changes to existing functionality, such that existing test suites need to be modified.
    • New functionality, such that new test suites will be needed.
      
      If either of these is the case, the "T" (Test Impact) button should be selected in your RTI.
      
      If you require new tests or significant modifications, please contact ON Test Sponsors and the RTI-test-review@sun.com before submitting your RTI. If you're not sure what constitutes a "significant" modification, please consult the same test sponsor group for clarification.
      
      If your changes are minor, file the appropriate bugs against the test suites and implement the change yourself. If implementing the change yourself is not practical, contact ON Test Sponsors and the RTI-test-review@sun.com to discuss potential assistance. List the CR number(s) in the "Comments" section of the RTI, and discuss the regression testing implications with your CRT advocate.
      
      Please do not submit your RTI until the relevant test changes are also committed. Your RTI should not be approved until the tests are ready.
      
       You and and your advocate can use your best judgment on whether automated testing is appropriate for your fix.
• The following "paperwork items" in the bug (CR) reports should be done:
  • The Description text should contain, surprisingly enough, a description of the problem, which should be at least as detailed as the Synopsis, and preferably much more. Note in particular that "See Comments" alone is never appropriate. While proprietary data (Sun confidential or customer confidential) must not be placed in the Description text, as much data as possible should be placed there. For security issues where there is a temporary need for secrecy until the problem is made public, put something vague then plan on updating it to be specific once the issue is made public. For issues involving proprietary customer information, either redact any proprietary information or move it to the Comments text, and write a high-level description which does not contain proprietary data.
  • The Status field should not be 1-Dispatched. 7-Fix in Progress is usual, though 5-Cause Known, 6-Fix Understood and even 8-Fix Available are fine too.
  • For Defects (as opposed to RFEs), the Introduced in Release/Build and Root Cause (under the More Info tab) fields must be filled in. Note that the Introduced in Release/Build field's value should be that of a marketing release (e.g., snv_01), not that of an update release. Also, if a bug ID exists for the changes(s) which introduced this problem, that/those bug ID(s) should be listed in the See Also field. If the bug has been around "forever", using "solaris_2.0" as an Introduced in Release is acceptable.
  • With respect to the Suggested Fix text, some very helpful suggestions follow, though of course common sense should always be applied:
    • A brief high-level description of the changes is good, especially if the diffs won't fit and need to be attached.
    • Each source module's full path (e.g., usr/src/.../foo.c) should be specified in the Suggested Fix text. Further, this text field should contain the diffs for each affected module. Context diffs (diff -c
      or diff -u
      , or sccs diffs -C or hg pdiff) are preferred if they will fit. If the diffs won't fit in one note entry in Bugster, try splitting them across mutiple entries of the Suggested Fix (for example, one file per entry). If they are so large that doing so would be overly laborious, then include a list of files that are impacted by this CR, attach full diffs to the CR and make sure you include a webrev in the RTI comments.
    • Note that the diffs should be in a permanent form, such as described above. A URL pointing to a webrev or any other transient data is allowable, but insufficient for a bug report, as the data in the bug report is expected to last, whereas webrevs and other URLs often disappear shortly after integration.
    • The diffs should be pointing the "right" way. webrev, sccs diffs and hg pdiffwill get this right; using diff from the command line, make sure you do diff old new
      .
    • When fixing multiple bugs as part of single wad, list the minimal set of diffs whenever possible. Sometimes the fixes are too intertwined to list separately, which is OK, but when that is not the case, listing minimal diffs makes it easier to understand each fix separately.
  • If your changes require corresponding changes in Solaris man pages or documentation, then you should also make sure that:
    • The Fix Affects Documentation field (under the More Info tab in the bug report) should be set to yes to match.
    • File man page and/or documentation bugs as needed, and list those bug IDs in WebRTI.
    • The corresponding man page bug IDs should be listed in the See Also field of the engineering bug report.  Furthermore, WebRTI requires that man page bugs be in the 7-Fix in Progress state and other documentation bugs to be in the 3-Accepted State before an RTI can be accepted. The man page group therefore recommends:
    • Filing man page bugs 4 builds ahead for moderately large projects, 2 builds ahead for minor changes to one or more man pages.
    • If your change needs to be integrated in the current build, file the man page bug as soon as you know that your code fix requires man page changes.
    • Include specific instructions regarding how to fix the man page in the man page bug.
    • Cite the associated engineering bug in the man page bug.
    • Notify the man page representative to the C-Team, currently Terry Gibson, that your man page bug is urgent. He will ensure that the man page receives immediate priority, or that you receive an exemption to putback your code without the man page CR being in the 7-Fix in Progress state.
  • The Synopsis should be a concise description of the problem, free from references to a specific build or release (unless only that build or release is impacted), and free from spelling and grammatical errors. Most of all, it should make sense. As the CR is investigated and the issue is better understood, the Synopsis should be updated to make sure it remains an accurate synopsis of the issue being addressed. This text often appears later in patch README files and other externally consumable formats.
• Output of dry run command to commit your changes
  • For Nevada builds 97 and later, the RTI should have your hg outgoing -v in the putback -n section and a pointer to your webrev in the comments section. Note that the user: entry should contain a valid e-mail address (i.e., user@domain, where domain is valid both inside and outside the SWAN). Note also that this user: shows up as an Author: entry in the `hg push` e-mail. This value can be set using the username= line in the [ui] section of your $HOME/.hgrc file.
  • For Solaris 10 updates and older sustaining releases, the RTI should also have your `putback -n` in the putback -n section and a pointer to your webrev in the comments section. Note also that if you run `putback -n` on the gate machine (which is allowed), the path to your child workspace (which you should include in the RTI) will be sufficiently qualified for an RTI advocate (who will often inspect your workspace) to find it.
• The list of code reviewers should be individuals who have actually reviewed the code, not an alias for a group of people who were given the opportunity to do so.
• Other more general nits:
  • No bug IDs in the source code; the sccs delta comments or the Mercurial changeset descriptions cover that.
  • For Nevada build 97 and later, there should be no merge changesets; hg pbchk should be clean.
  • For Solaris 10 and earlier sustaining releases, each source file should add only a single delta, without "merge turds". In the case where a putback includes multiple bug fixes and/or ARC cases, multiple deltas are allowed, but there should still only be a single delta per bug fix and/or ARC case.
• Here is an explanation of what the Risk and Impact fields are for:
  • Risk assesses how much confidence that we have that the fix will not introduce other problems, and a component of how easy it would be to avoid them problems if they occur.
  • Impact is somewhat akin to external side-effects: the customer will no longer see the bug, but will notice something else in an adverse way.
    • API changes might stop an application from working.
    • Documentation changes can have subtler effects.
    • Header file changes may cause code compilation problems. Note that e.g. if you have a private header file and you fix all of the consumers, that does not count as external, so you need not mark Header impact. But header files consumed by other projects generally merit Header impact and updating a header file which is shipped in /usr/include always merits Header impact.
    • Packaging / namespace may remove something they rely on if they have built-in paths
    • Testing changes means one or more test suites need to be added or updated, and the CR(s) for such changes should be listed in the comments section of the RTI.
    • "Other" includes everything else and we require details in the comment section of the RTI to say what it is. Examples of other effects are:
      • slower
      • uses more memory
      • change in messages not covered by documentation changes
      • causes a flag day
      • requires a coordinated putback in a different consolidation  An Impact of none is the default case: The bug got fixed is all that the customer can detect. If Impact is not none, it should always be explained in the RTI comments.

Reviewers

• Naked strcpy(), strcat() and sprintf() calls should be avoided wherever possible; use strlcpy(), strlcat() and snprintf() respectively instead.
• You should not be listed as a reviewer for your own RTI: the point is that someone else should review your code.
• It is OK to have reviewers who are not Sun employees. Advocates should treat them the same as Sun employees, making judgements based on how well one knows each reviewer and the history and quality of his/her work. When such reviewers are listed in WebRTI, the e-mail notices do go outside Sun, so take care in such cases to avoid putting any proprietary or confidential data in any comments. [Such data is rare in RTIs; if needed, move it to the Comments note of the bug report(s) and put a pointer to that in the RTI.]

Advocates

• You can not be the advocate for your own RTI: the point is that someone else should review your work. Furthermore, if you are listed as one of the reviewers, then make sure that there is at least one other reviewer (or two in the builds leading up to a milestone) besides yourself.
• Workspace checking is highly recommended, as it is easy and quick and has prevented many problems; also, many problems which could have easily been prevented were from workspaces which were not checked.
• Check that the "user" in the hg outgoing -v output to make sure it is a valid email address that belongs to the user doing an integration.

Sponsors for External Contributors

• A Sun Contributor Agreement (SCA) must be on file for the contributor.
• You can check the request-sponsor table to see if the SCA number is listed for the contributor (it usually is). If not, check with the contributor to sure s/he filed the form and find out the number. There is also an internal table available that can be checked. If you don't know where this table is located, check with oso-admin@opensolaris.org.
• The rest of the "sponsoring nits" are on the Sponsor Tasks page.