On Wed, Sep 15, 2021 at 02:31:28AM +0200, Ævar Arnfjörð Bjarmason wrote: > I haven't actually run this yet (and need to zZzZ soon), but AFAICT at > the end of this series you leave the existing advertise semantics of > advertise() be (which is fine). I have this unsubmitted patch locally > which you may or may not want to work into this. > > I considered splitting up the advertise() method as well, i.e. we could > have a new "is_advertised" boolean callback, and then a > "capability_string" or whatever. "server-option" and "object-info" never > add anything, so they could leave that as NULL. > > But it's probably not worth it, just food for thought... Yes, I noticed the advertise() function is really doing double-duty here. Nothing changes with respect to it in my series, but I agree it's a bit confusing. The functions are simple enough that splitting up the two functions of advertise() might make sense. Likewise, your documentation seems reasonable. I'd prefer to punt on it for now, though, as my series isn't otherwise touching this function. I'm sure there are textual conflicts with what I'm doing here, since it's nearby, but I'd prefer to just build more cleanups on top later. -Peff
