Le 29/08/12 23:55, Sumana Harihareswara wrote:
- Write small commits.
I cant stress how important this is. git has several ways to split a commit: - git rebase --interactive <parent commit sha1> - reset to master and git cherry-pick --no-commit <sha1> then use git add --patch to select the hunk to craft a new small commit. --> http://nuclearsquid.com/writings/git-add/
<snip>
- Don't mix rebases with changes.
When rebasing, only rebase. That makes it easier to use the "Old Version History" dropdown, which greatly quickens reviews. If non-rebase changes are made inside a rebase changeset, you have to read through a lot more code to find it and it's non-obvious.
The way to go is usually to git rebase and send that then do the new change. That make reviewing ten times easier.
It is also a good practice to add a cover message on the new patchset to explain what it changes compared to the previous one. For example:
PS3: rebased on latest master PS4: fix the, now obsolete, function() call
Where PS is used instead of "PatchSet".
- Add reviewers.
If you do not know who could review your change, do ask in #mediawiki or #wikimedia-dev. People with a long experience will be able to tell you who might be able to help.
- Review more.
Asking a question about code, commenting about anything (whitespaces, code you do not understand or dont like), doing +1 / -1, is NEVER going to harm anyone. I have seen reviews from newbie that were not understanding some part, and it ended up being a bug. Reading code is a great way to learn the framework, asking question is even better :)