Logitech#765 - Provide support for npm cometd client #767
Conversation
…at server echo 'id' fields from 'meta/*' messages to support callbacks and notifications.
There was a problem hiding this comment.
That change makes sense - thanks a lot! Please modify as per my comment. I'll be happy to merge to 8.3 first, then potentially back port it later (when confirmed working in 8.3). There's no need for two pull requests. They's eventually conflict with each other.
| my $msgid; | ||
| if ( !$msgid ) { | ||
| # obtain 'id' from outer object if present, otherwise default to - Here we use different variable name '$msgid' as '$id' is defined for use within slim/subscribe channel. | ||
| if ( $obj->{id} ) { | ||
| $msgid = $obj->{id}; | ||
| } else { | ||
| #default to empty string if not available. | ||
| $msgid = ''; | ||
| } | ||
| } |
There was a problem hiding this comment.
$msgid will always be falsy right after definition. This block could be simplified to the following, couldn't it?
my $msgid = $obj->{id} || '';
There was a problem hiding this comment.
Never mind, I'll do this on my end. Thanks!
There was a problem hiding this comment.
Yes Michael. that is perfect and what I'd have done if I knew how.
I'm a VisualBasic, C/C++, Java, NodeJS, Fortran, C#, .NET guy.
I dabbled in Perl and Python early on and to get the right syntax, I Google.
While working on this I considered the following:
In the /slim/request section we could move the my $id variable to the top and use it instead of my $msgid. That way there's no confusion and $id means $id.
Please double-check my work for all the error conditions to make sure I did not screw things up.
Provide support for npm cometd client that requires that server echo 'id' fields from 'meta/*' messages to support callbacks and notifications.