Closed GoogleCodeExporter closed 8 years ago
Original comment by teichsta
on 10 Apr 2013 at 7:15
* all classes (especially those beneath internal.items) should contain (at
least) class level javadoc
* all classes should contain @author tag
* all classes should contain @version 1.2.0 tag
* openhab_default.cfg should contain default parameter and comments (see mpd
section)
* remove unused code (sys.out, etc.)
* remove Main.java
# Parser
* please document the various parse methods in more detail (javadoc)
# PulseaudioBindingProvider
* Item should not be used as return type since the Item itself is not really
used here. It is more the items type which is relevant. It would be better to
just return the type (Class<? extends Item> see HttpBindingProvider)
# PulseaudioBinding
* remove references (member, setter, getter) to EventPublisher since it is
already contained in AbstractBinding superclass
* execute() and interncalReceiveCommand() are really looooooong methods. It
would be good to split them to increase readability but better not before 1.2.
Just put it on the TODO-list and refactor the code afterwards ;-).
## activate()
* decrease log level to debug (or remove logging here)
##deactivate()
* decrease log level to debug (or remove logging here)
# PulseaudioClient
* remove unused code
* move code from finalize() to explicit disconnect method and execute that
method from PulseaudioBinding.deactivate()
* all Strings which cointains relevant "commands" should be extracted as static
member and should be documented
* add more documentation
# PulseaudioGenericBindingProvider
* implement validateItemType()
# PulseaudioCommandTypeMapping
* remove unused code
# PulseaudioMonitor
* better letter PulseaudioBinding extends AbstractActiveBinding
* move execute code to PulseaudioBinding
* remove class
* remove pulseaudiomonitor.xml
* remove reference to pulseaudiomonitor.xml from MANIFEST.MF
After all: good work :-)
Thanks for this contriution!!
Original comment by teichsta
on 10 Apr 2013 at 7:57
Ok thank you for the hints. I have done all the suggested changes, except the
following:
* all classes should contain @version 1.2.0 tag
Do you mean the @since 1.2.0 tag -> I have changed that, I did not see a
@version tag in any other binding I looked into
* remove unused code (sys.out, etc.)
I did not found unused files like sys.out in my binding. All code that I had
commentet out has been removed.
* execute() and interncalReceiveCommand() are really looooooong methods. It
would be good to split them to increase readability but better not before 1.2.
Just put it on the TODO-list and refactor the code afterwards ;-).
Well as you suggested I put this on my todo list after 1.2.0 release
I did i short test of the changed binding in my production system and
everything seems to be ok, so from my point of view its ready to be merged.
Original comment by TBraeuti...@gmail.com
on 10 Apr 2013 at 12:36
wow - that's fast :-)
* yes, i meant the @since tag, sorry!
* by "sys.out" i meant "lines" like "//System.out.println("bla")", so
everything is fine
Thanks a lot, will merge asap.
Regards,
Thomas E.-E.
Original comment by teichsta
on 10 Apr 2013 at 12:43
merged to default (see
http://code.google.com/p/openhab/source/detail?r=692cbbbf5260b8b23218de252b81c81
2d797cbe2)
thanks again for this contribution!!
Original comment by teichsta
on 10 Apr 2013 at 7:19
Original issue reported on code.google.com by
teichsta
on 10 Apr 2013 at 7:14