Difference between revisions of "Defect and Feature Review Process"

From OpenSSLWiki
Jump to navigationJump to search
(Added proposed status first line)
m (added not to use PR#NNNN format and to include git pull URL ... that way automation scripts can pick things up quickly)
 
(2 intermediate revisions by one other user not shown)
Line 3: Line 3:
 
==Principles==
 
==Principles==
  
* Start looking at the newest entries in RT first and work backwards in time. This is on the basis that the newest bugs are likely to still be present and most relevant. As we go back in time we will see more and more which are no longer relevant - we will start hitting the law of diminishing returns.
+
* Start looking at the newest tickets in RT first and work backwards in time. This is on the basis that the newest tickets are likely to still be present and most relevant. As we go back in time we will see more and more which are no longer relevant - we will start hitting the law of diminishing returns.
  
* Older entries can be looked at on an ad-hoc basis if a person doing triage identifies them (probably on the basis of some post to the dev list) as being important
+
* Older tickets can be looked at on an ad-hoc basis if a person doing triage identifies them (probably on the basis of some post to the dev list) as being important
  
* Any new RT entries coming in should get the very highest priority. We can only start to make progress if the problem doesn't continue to grow.
+
* Any new RT tickets coming in should get the very highest priority. We can only start to make progress if the problem doesn't continue to grow.
  
 
==Triage==
 
==Triage==
  
New defects start off in the "New" status. Someone assigned to triage duty will do a first pass assessment about what the RT entry is about.
+
New tickets start off in the "New" status. Someone assigned to triage duty will do a first pass assessment about what the ticket is about.
  
1) The person triaging the report classifies it as one of:
+
1) The person triaging the report assigns a severity:
  
* Bug report
+
* Wishlist - For all feature requests
* Feature request
+
* Blocker - Liable to cause inability to use an important feature for a large number of users
 +
* Critical - Liable to cause inability to use an important feature for a small number of users
 +
* Normal - the default level. Something doesn't work right
 +
* Nice to have - a minor defect which does not affect the fundamental use of OpenSSL or where there are simple work arounds
  
 
And gives it a status of one of:
 
And gives it a status of one of:
  
* Closed (the report has already been dealt with; or the report is not relevant or appropriate)
+
* Resolved (the report has already been dealt with)
 +
* Rejected (the report is incorrect, not relevant or is not appropriate. Can also be used for defects that will not be fixed, or feature requests that will not be implemented)
 
* Open (the report appears to be sane from reading it and requires further investigation)
 
* Open (the report appears to be sane from reading it and requires further investigation)
  
The triage person classifies the report by area and puts it on one of the following queues:
+
The triage person classifies the ticket by area and assigns it to a subsystem. Some example subsystems are:
* Documentation
+
* Doc - For all documentation
* Command line app
+
* Apps - All the command line apps
* libssl
+
* TLS/SSL - Anything related to libssl
* libcrypto
+
* Build - Anything related to compilation & installation
* Compilation & installation
+
* Other - Anything that doesn't fit neatly into any other category
* Platform specific
+
 
* Other
+
Finally the triager person should determine how quickly this ticket should be resolved by setting the Milestone release that it should be targeted for.
  
 
==Report Confirmation and Approval==
 
==Report Confirmation and Approval==
  
Someone reviewing a queue will take ownership of a report (keeping in mind the principles stated above). It is assumed that people will review the queues that they have most expertise in:
+
Someone reviewing open tickets will take ownership of one of them (keeping in mind the principles stated above). It is assumed that people will review tickets associated with the sub-systems that they have most expertise in:
  
2) A defect owner will pick up Open requests from a queue and mark them as "owned" by them. They then attempt to recreate the bug (if appropriate). The status is then updated to be either:
+
2) A defect owner will pick up Open tickets and mark them as "owned" by them. They then attempt to recreate the bug (if appropriate). The status is then updated to be either:
  
* Confirmed
+
* Confirmed - [NOTE THIS STATUS DOES NOT CURRENTLY EXIST - USE THE OPEN STATUS AND CREATE A COMMENT STATING THAT THE DEFECT IS CONFIRMED FOR NOW]
* Not Confirmed (this is effectively a closed status - it could be reopened if the initial person reporting the defect provides further information)
+
* Rejected (it could be reopened if the initial person reporting the defect provides further information)
  
 
3) Review or create patch
 
3) Review or create patch
  
3a) If the bug is confirmed and a patch has been supplied then the owner verifies that the patch fixes the issue. They can also sanity
+
3a) If the bug is confirmed and a patch has been supplied then the owner:
check the patch to make sure it looks reasonable. If all is ok the owner should also check that the patch can be applied to all branches. If not the owner can either port the patch themselves to all branches, or request that the submitter do it. Once all of this is done the owner loads the patch into their github repository and creates a pull request (including the RT number in the request)
+
* Verifies that the patch fixes the issue
 +
* Sanity check the patch to make sure it looks reasonable
 +
* If all is ok the owner should also check that the patch can be applied to all relevant branches
 +
* If not the owner can either port the patch themselves to all branches, or request that the submitter do it
 +
* Once all of this is done the owner loads the patch into their github repository and creates a pull request (including the '''RT number''' in the request with a comment of PR#NNNN)
 +
* Once the pull request is created, add a comment to the ticket with the pull request details in the form of the URL(s)
  
 
The status is updated to either:
 
The status is updated to either:
* Rejected (the patch is not suitable, appropriate, or available for all branches. Again this could be reopened if the original poster submits a revised patch)
+
* Stalled (the patch is not suitable, appropriate, or available for all branches. Again this could be reopened if the original poster submits a revised patch)
* Approved (the patch is in github for all branches and appears sane - it is ready for the dev team to review)
+
* Approved (the patch is in github for all branches and appears sane - it is ready for the dev team to review) [NOTE THIS STATUS DOES NOT CURRENTLY EXIST - USE THE OPEN STATUS AND CREATE A COMMENT STATING THAT THE DEFECT IS CONFIRMED FOR NOW]
  
 
3b) If the bug is confirmed and no patch is available then the same process as above applies, but the owner creates the patch themselves.
 
3b) If the bug is confirmed and no patch is available then the same process as above applies, but the owner creates the patch themselves.
  
4) The dev team only look at Approved RT reports and verify that they are happy with the patch before committing it.
+
4) The dev team only look at Approved RT tickets and verify that they are happy with the patch before committing it.
  
 
The status is updated to either:
 
The status is updated to either:
 
* Rejected (as above)
 
* Rejected (as above)
* Closed
+
* Resolved

Latest revision as of 00:08, 30 April 2014

THIS IS A PROPOSED PROCESS

Principles[edit]

  • Start looking at the newest tickets in RT first and work backwards in time. This is on the basis that the newest tickets are likely to still be present and most relevant. As we go back in time we will see more and more which are no longer relevant - we will start hitting the law of diminishing returns.
  • Older tickets can be looked at on an ad-hoc basis if a person doing triage identifies them (probably on the basis of some post to the dev list) as being important
  • Any new RT tickets coming in should get the very highest priority. We can only start to make progress if the problem doesn't continue to grow.

Triage[edit]

New tickets start off in the "New" status. Someone assigned to triage duty will do a first pass assessment about what the ticket is about.

1) The person triaging the report assigns a severity:

  • Wishlist - For all feature requests
  • Blocker - Liable to cause inability to use an important feature for a large number of users
  • Critical - Liable to cause inability to use an important feature for a small number of users
  • Normal - the default level. Something doesn't work right
  • Nice to have - a minor defect which does not affect the fundamental use of OpenSSL or where there are simple work arounds

And gives it a status of one of:

  • Resolved (the report has already been dealt with)
  • Rejected (the report is incorrect, not relevant or is not appropriate. Can also be used for defects that will not be fixed, or feature requests that will not be implemented)
  • Open (the report appears to be sane from reading it and requires further investigation)

The triage person classifies the ticket by area and assigns it to a subsystem. Some example subsystems are:

  • Doc - For all documentation
  • Apps - All the command line apps
  • TLS/SSL - Anything related to libssl
  • Build - Anything related to compilation & installation
  • Other - Anything that doesn't fit neatly into any other category

Finally the triager person should determine how quickly this ticket should be resolved by setting the Milestone release that it should be targeted for.

Report Confirmation and Approval[edit]

Someone reviewing open tickets will take ownership of one of them (keeping in mind the principles stated above). It is assumed that people will review tickets associated with the sub-systems that they have most expertise in:

2) A defect owner will pick up Open tickets and mark them as "owned" by them. They then attempt to recreate the bug (if appropriate). The status is then updated to be either:

  • Confirmed - [NOTE THIS STATUS DOES NOT CURRENTLY EXIST - USE THE OPEN STATUS AND CREATE A COMMENT STATING THAT THE DEFECT IS CONFIRMED FOR NOW]
  • Rejected (it could be reopened if the initial person reporting the defect provides further information)

3) Review or create patch

3a) If the bug is confirmed and a patch has been supplied then the owner:

  • Verifies that the patch fixes the issue
  • Sanity check the patch to make sure it looks reasonable
  • If all is ok the owner should also check that the patch can be applied to all relevant branches
  • If not the owner can either port the patch themselves to all branches, or request that the submitter do it
  • Once all of this is done the owner loads the patch into their github repository and creates a pull request (including the RT number in the request with a comment of PR#NNNN)
  • Once the pull request is created, add a comment to the ticket with the pull request details in the form of the URL(s)

The status is updated to either:

  • Stalled (the patch is not suitable, appropriate, or available for all branches. Again this could be reopened if the original poster submits a revised patch)
  • Approved (the patch is in github for all branches and appears sane - it is ready for the dev team to review) [NOTE THIS STATUS DOES NOT CURRENTLY EXIST - USE THE OPEN STATUS AND CREATE A COMMENT STATING THAT THE DEFECT IS CONFIRMED FOR NOW]

3b) If the bug is confirmed and no patch is available then the same process as above applies, but the owner creates the patch themselves.

4) The dev team only look at Approved RT tickets and verify that they are happy with the patch before committing it.

The status is updated to either:

  • Rejected (as above)
  • Resolved