Difference between revisions of "Defect and Feature Review Process"
(Added proposed status first line) |
(Significant updates based on RT experience) |
||
Line 3: | Line 3: | ||
==Principles== | ==Principles== | ||
− | * Start looking at the newest | + | * 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 | + | * 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 | + | * 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 | + | 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 | + | 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: | 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) | * Open (the report appears to be sane from reading it and requires further investigation) | ||
− | The triage person classifies the | + | 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 |
− | * libssl | + | * 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== | ==Report Confirmation and Approval== | ||
− | Someone reviewing | + | 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 | + | 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] |
− | * | + | * 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 | + | 3a) If the bug is confirmed and a patch has been supplied then the owner: |
− | check the patch to make sure it looks reasonable | + | * 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) | ||
+ | * Once the pull request is created, add a comment to the ticket with the pull request number | ||
The status is updated to either: | 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) | + | * 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 | + | 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) | ||
− | * | + | * Resolved |
Revision as of 23:15, 29 April 2014
THIS IS A PROPOSED PROCESS
Principles
- 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
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
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)
- Once the pull request is created, add a comment to the ticket with the pull request number
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