Hi. On Feb 15, 2011, at 10:28 AM, Felix Kaiser wrote:
> Hi, > > a) If I reviewed a pull request, can I push it in directly or should I let > one of the core devs do it? Yes, please do. You can especially push stuff in for the webpage (because that isn't going to "break any tests". For the main repo, as long as you are sure that the tests all pass and that there are no objections from anyone, you can also do so. Pushing stuff in when it has been reviewed really helps us, so this is actually encouraged (otherwise we get behind). So basically, there are three main things to do before pushing (am I forgetting anything anyone?): 1. Make sure all tests pass (if it is part of the main repo; for the webpage, make sure everything looks good and also that the pages are built). Do this for the merged/rebased version to make sure that that didn't introduce any errors. 2. Make sure that all content has been reviewed (for example, make sure that the person didn't push in a new commit to the branch for review that you didn't notice). 3. Make sure that no one else has any objections to the branch. Everything is based on consensus, so until one is reached, the branch cannot be pushed in. > > b) Should commits in pull requests be rebased onto master and, if sensible, > squashed before they are pushed in? If yes would you, for example, squash the > commits in Vinzents Logo pull request[1]? > This is up to your discretion. For example, I have already pushed that branch in, using the merge method, because that was the easiest way to do it. But I could have just as well done a rebase. Each has its advantages and disadvantages. A good rule of thumb is that rebasing is better for small branches (one or two commits), and merging is better for very large branches (a dozen or more commits, or especially if there are more on the order of a hundred commits). Of course, if there are conflicts, you might want to leave it to the decision of the original commit author to do this for you (unless you know how to solve them). > Greetings > Felix > > [1] "Logo" pull request: https://github.com/sympy/sympy.github.com/pull/14 -- You received this message because you are subscribed to the Google Groups "sympy" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/sympy?hl=en.
