Code review comment for lp:~ipython-contrib/ipython/ipython-zmq

Revision history for this message
Fernando Perez (fdo.perez) wrote :

I merged revisions 1237-1239 and cleaned the text up a little bit, but I did NOT merge 1240-41. The code in there will require further review, and we're now doing the move to github.

So once you start working on github, just remember to add the ipzmq.py file back to your git branch. But that code will need some fixes, which I can tell you about right away so you can start working on them:

- you are using 8 spaces, PEP 8 uses 4
- you are using camelCase, PEP 8 discourages that. Please read PEP 8 and adhere to it.
- do not use self.__X attributes with double underscore, they cause lots of problems in the long run. They are a mis-feature of python and should only be used in rare instances.

- Why aren't you using the code from the pyzmq examples as a starting point? That code already had much of what you are implementing here working...

So in summary: your docs have been merged (after fixing for capitalization and formatting, next time please be careful with that). The code isn't ready for merging.

review: Disapprove

« Back to merge proposal