Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

FIX extra comma in Sankey.add #1472

merged 1 commit into from Nov 13, 2012


None yet
2 participants

NelleV commented Nov 9, 2012

There was a mistake in one of my PEP8 patches: I added an extra comma. Hence, the code failed when calling it.

I fixed the error, and added a small test that just checks the code runs.


dmcdougall commented Nov 9, 2012



dmcdougall commented Nov 11, 2012

It looks like you also moved the __init__ function to the top of the class. While I actually prefer that, I think I'd also prefer that change to be in its own commit.

My feeling is that the commit message should reflect the changes that are made. The commit message currently says you fixed an issue with a comma, but the init function was also moved.

Sorry to be picky -- but it does make the logs and diffs easier to read.


dmcdougall commented Nov 11, 2012

Also if anyone else disagrees, feel free to shoot me down.


NelleV commented Nov 13, 2012

I've added this to the commit message

@dmcdougall dmcdougall added a commit that referenced this pull request Nov 13, 2012

@dmcdougall dmcdougall Merge pull request #1472 from NelleV/test_sankey
FIX extra comma in Sankey.add

@dmcdougall dmcdougall merged commit c6221e2 into matplotlib:master Nov 13, 2012

1 check passed

default The Travis build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment