Gary Poster has proposed merging lp:~gary/juju-gui/process into lp:juju-gui.
Requested reviews: Juju GUI Hackers (juju-gui) For more details, see: https://code.launchpad.net/~gary/juju-gui/process/+merge/132493 Add more review docs Docs for starting a branch, preparing a branch for review, and reviewing. Self-approved. -- https://code.launchpad.net/~gary/juju-gui/process/+merge/132493 Your team Juju GUI Hackers is requested to review the proposed merge of lp:~gary/juju-gui/process into lp:juju-gui.
=== modified file 'docs/process.rst' --- docs/process.rst 2012-10-05 09:42:27 +0000 +++ docs/process.rst 2012-11-01 10:08:31 +0000 @@ -2,21 +2,79 @@ Process Notes ============= +Checklist for Starting a Branch +=============================== + +- Understand the problem. If you don't, ask and be persistent until you do. +- When determining a solution, consider this preferred Software + Architecture Cheat Sheet: + http://gorban.org/post/32873465932/software-architecture-cheat-sheet +- Have a pre-implementation call with someone about your solution. If you + are not sure of your solution, prototype before the pre-implementation call. +- Use TDD. Your prototype might be perfect, but you can still move it aside + and rebuild it gradually as you add tests. It can go quickly. + +Checklist for Preparing for a Review +==================================== + +- Round-trips with reviewers are expensive. Try to eliminate them. + * Pre-review your diff! Tip: you can diff your branch against a local + trunk named "trunk" with "bzr diff -r ancestor:../trunk/". + ~ All new code should have tests. If it doesn't, be prepared to explain + why to skeptical reviewers. + ~ Have you cleaned out temporary comments and debugging changes? + ~ Is the code understandable? + ~ Do you have superfluous or duplicated code? + * Run tests! Someday we'll have that in the lbox hook... +- Make sure there is a bug for your branch. Ideally there was one when you + started, but if not, see the "-new-bug" option to "lbox propose". +- Aim for a branch diff size between 300 and 500 lines. +- Treat branch diff sizes of more than 800 lines as a problem. + ~ Try to subdivide it now. If you can't, explain to your reviewers your + reasoning. + ~ Treat this as an opportunity to learn. Consider what you could have + done differently. +- If the branch is very minor, such as a documentation change, feel free to + self-review. However, *don't neglect to consider your responsibilities + above*, especially the diff review and running tests (even if you think + there's no way the tests could have been affected). + +It is your responsibility to get reviewers of your branch! Reviewers will +hopefully take it, but if they don't, rustle some up. + +When you get your reviews back, be appreciative, and be sure to respond to +every request, even if it is to disagree. + +Once you have two approving reviews (and no disapproving reviews), you may +land your branch. + Checklist for Reviewing ======================= -- Get an idea of what the branch is doing from the diff. -- Run ``make test`` and confirm that tests pass. (This step can be removed once - we have tarmac.) -- Run ``make lint`` and confirm that the output is clean. (This step can be - removed once we have tarmac.) -- Run ``python improv.py -f sample.json`` in the rapi-delta juju branch, - and run ``make server`` with the juju-ui branch. QA the changes if - appropriate, and then do some exploratory testing to make sure - everything seems to work. For extra points, try a few different - browsers. TODO: Document some manual QA scripts for some of the basic - paths. +- Run ``make test`` and confirm that tests pass. +- Run ``python improv.py -f sample.json`` in the rapi-rollup juju branch, and + run ``make server`` with the juju-ui branch. + * Don't forget to clear the browser cache: index.html may be sticking around + because of the cache.manifest. + * QA the changes if possible, exploring different use cases (and edge cases). + * Spend between 60 and 120 seconds exploring the entire app. Do different + things every time. Try to break the app, generally. +- [Once we support multiple browsers, try them all, at least briefly.] - Review the diff, including notes from the above as appropriate. + * Make sure that new code has tests. + * Make sure you can understand the new code. Ask for clarification if + necessary. + * Consider the advice in this preferred Software Architecture Cheat Sheet. + http://gorban.org/post/32873465932/software-architecture-cheat-sheet + * Make sure changes to a file correspond to the naming conventions and other + stylistic considerations local to that file, and within our project. + * Before you ask for a change, think and make sure you can't compromise + reasonably with the coder. If there is a low importance disagreement, in + general the coder's position should win. +- In your summary message, thank the coder. +- In your summary message, if you ask for changes, make it clear whether you + want to re-review after the changes, or if you automatically approve if the + changes are made. Checklist for Running a Daily Meeting ===================================== @@ -123,7 +181,8 @@ making yourself a more valuable employee to Canonical (i.e., studying a technology that is important to the company), improving processes or tools for our team, or building or improving something for another part - of Canonical. - Consider who you expect to maintain the project. + of Canonical. +- Consider who you expect to maintain the project. - Yourself: Be skeptical of this, but if so, that's fine. - Our team: discuss design with team, and/or follow the "prototype, discuss,
-- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
