search

geronimp / graftM

GraftM - Rapid community profiles from metagenomes
http://geronimp.github.io/graftM/
GNU General Public License v3.0
44 stars 16 forks source link

Dendropy reannotator #206

Closed wwood closed 8 years ago

wwood commented 8 years ago

Hey,

Now passing all the rerooting tests and all other nosetests too.

This should be backwards-compatible because Reannotator._reroot_tree_by_old_root has the same interface.

It actually makes more sense not to do things the 'pull request way' because the two commits do individual things - perhaps makes more sense to go through the commits separately and annotate anything you find in the commits themselves: e.g. write comments on these pages: https://github.com/geronimp/graftM/commit/23c9afbd9d6348b1e912ce0ed37f1b9366afb7ef https://github.com/geronimp/graftM/commit/636252d8b81ca41171f3bbc9406b60c703448566

because then you can see exactly what changed in the 2nd commit rather than seeing the move+change at the same time which is what you see if you just look at the '5 changed files'.

thanks for the review.

wwood commented 8 years ago

Hey I fixed that bug I was talking about now, would you mind reviewing please? Thanks.

geronimp commented 8 years ago

Hey, did you commit the bugfix?

wwood commented 8 years ago

yes, with git commit --amend, git push -f, there's no extra commit

geronimp commented 8 years ago

Okay, I see

Joel

On 18 May 2016 at 11:35, Ben J Woodcroft notifications@github.com wrote:

yes, with git commit --amend, git push -f, there's no extra commit

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/geronimp/graftM/pull/206#issuecomment-219902178

wwood commented 8 years ago

shall I merge?