On Tue, Apr 5, 2022 at 6:21 AM Peter Smith <smithpb2...@gmail.com> wrote: > > Here are my comments for the latest patch v6-0001. > > (I will post my v6-0002 review comments separately) > > PATCH v6-0001 comments > ====================== > > 1.1 General - Option name > > I still feel like the option name is not ideal. Unfortunately, this is > important because any name change would impact lots of these patch > files and docs, struct members etc. > > It was originally called "local_only", but I thought that as a > SUBSCRIPTION option that was confusing because "local" means local to > what? Really it is local to the publisher, not local to the > subscriber, so that name seemed misleading. > > Then I suggested "publish_local_only". Although that resolved the > ambiguity problem, other people thought it seemed odd to have the > "publish" prefix for a subscription-side option. > > So now it is changed again to "subscribe_local_only" -- It's getting > better but still, it is implied that the "local" means local to the > publisher except there is nothing in the option name really to convey > that meaning. IMO we here all understand the meaning of this option > mostly by familiarity with this discussion thread, but I think a user > coming to this for the first time will still be confused by the name. > > Below are some other name ideas. Maybe they are not improvements, but > it might help other people to come up with something better. > > subscribe_publocal_only = true/false > origin = pub_only/any > adjacent_data_only = true/false > direct_subscriptions_only = true/false > ... >
I think local_only with a description should be okay considering other current options. Some of the options like slot_name, binary, etc. are publisher-specific which becomes clear only after reading the description. I am not sure adding pub*/sub* to it is much helpful. So, +1 to a simple boolean like 'local_only', 'data_local', or something like that. -- With Regards, Amit Kapila.