Woof! wrote: > On Wed, 15 Oct 2008 16:34:07 -0400, > <[EMAIL PROTECTED]> wrote: > > Changed the Expires value of the SUBSCRIBE from 1 to 4. > > I understand the interests of keeping a patch small, but all > throughout the code are comments like: > > > // If "1 second subscriptions" is set (needed for > some versions > // of Snom phones), use a 1-second subscription. > Otherwise, use > // a 0-second subscription, so we get just one NOTIFY. > subscribe.setExpiresField(mOneSecondSubscription ? 4 : 0); > > I'd like at least to see the comments change from "1 second" > to "4 seconds", and it would be even nicer if the method > variable would change as well as "mOneSecondSubscription" is > no longer a good descriptive name for what is going on. In > addition, it mentions this is a workaround for Snom phones, > and now this is an additional workaround for Polycom phones > that should be documented here. > > Super bonus points for changing the config from: > #define CONFIG_SETTING_1_SEC \ > "PICKUP_1_SEC_SUBSCRIBE" > > to something more descriptive of what it now does, although I > understand the issues with changing that touches way more > than it should!
I am guilty. This is a quick fix for 3.10.3 to tide us over until Polycom's fix for XTRN-231 becomes available. It should die in the 3.10.3 stream, and never see the light of main. Hence I didn't bother doing this change "right". How about I update the code comment to cite XECS-1706 and explain why 4 is used instead of 1. Is that a good comprimise? -Paul [EMAIL PROTECTED] _______________________________________________ sipx-dev mailing list [email protected] List Archive: http://list.sipfoundry.org/archive/sipx-dev Unsubscribe: http://list.sipfoundry.org/mailman/listinfo/sipx-dev
