major code changes - Githubissues

thomasleveil / b3-plugin-poweradminurt

B3 plugin that brings Urban Terror 4.1 and 4.2 admin features to B3

http://forum.bigbrotherbot.net/xlr-releases/poweradminurt-1-4-0-for-urban-terror!/

4 stars 2 forks source link

major code changes #16

Closed danielepantaleone closed 10 years ago

danielepantaleone commented 10 years ago

refactored plugin syntax to match PEP8 conding style guide
more verbose logging on plugin configuration
catch all raised exception instead of discarding them
correctly use config getFloat method when needed
log message consistency (used same pattern for plugin configuration log messages)
declare missing attributes
flagged some attributes with correct scope (protected)
correctly declare lists as lists instead of dictionaries
renamed variables using python reserved symbols
fixed forceteam %s spectate: spectate is not interpreted by the gamecode
added missing command descriptions
fixed some in-game message spelling
replaced color code ^9 with ^1 -> red in both 4.1 and 4.2
make use of self.console.setCvar when possible

thomasleveil commented 10 years ago

Nice move :smiley: , that code got old and needed some clean up

A few remarks however:

please keep only one fix/feature per commit. This rule is to make code review easier, especially when mixing formatting changes with fixes/new features.
run the automated tests suite. They prevent regressions (a first one is fixed in 22762337a, a second one is still to fix)

please merge the pull request I sent you and fix the last regression and I'll merge when all tests pass

To run the tests on Windows with Git Bash installed:

Thomas@THOMAS-PC /H/workspaces/workspace-python/b3-plugin-poweradminurt (fenix_huge_refactor)
$ PYTHONPATH="../big-brother-bot;extplugins" nosetests

danielepantaleone commented 10 years ago

Hei...Yeah I usually keep 1 fix per commit. I did everything together because it wasn't meant to pushed on git. I was making some modifications for a friend and then i started rearranging the code and it ended up with all the things here above.

danielepantaleone commented 10 years ago

Fixed everything. I wasn't and I'm not still able to run tests on Windows (import b3.plugins works but import b3.output doesnt....go figure). Anyway I made a new setup on my MacBook and all the tests were good

thomasleveil commented 10 years ago

perfect! thx a lot :+1: