Code review comment for lp:~alisonken1/openlp/pjlink2-q

Revision history for this message
Ken Roberts (alisonken1) wrote :

> Sorry will clash with one of my fixes which has been merged
>
> See question

>> 'ACKN': {'version': ['2', ],
>> + 'default': '2',

>Why do we have a missing slot?

2 new commands in PJLink are only valid for version 2 and have no version 1 equivalent.
Missing slot is to maintain compatibility with version checker.

>> - def process_clss(self, data):
>> + def process_clss(self, data, *args, **kwargs):

>why args
Some commands take host,port arguments but not all of them do.
Since there is a common entry point, need to keep availability of consuming extra args/kwargs without causing exception due to extra parameters.
(Yes, there should be no args passed only keyword args, but better safe than sorry)

« Back to merge proposal