Comment 5 for bug 278388

Revision history for this message
Mark Lee (malept) wrote :

In the future, could you mark your patches by checking "this attachment is a patch"? I prefer to view the patches in the browser, rather than download them. I've modified both of your current patches in this way.

Some style comments:

While I don't like the fact that the applet is currently using two-spaces-per-indentation (it should be following PEP8, which requires four spaces per indentation), that's what the convention is. Make sure your indentations are consistent. I think there are three inconsistencies.

You should probably rename the second parameter of GetPandoraUrl.__init__() because it's the same name as a module that you import.

Other than that, I'm going to defer review of the patch to someone who's more familiar with writing Python Awn applets. (Testers are also welcome to comment.)