TL;DR: Make qdoc openly document \internal members of documented classes/namespaces or defined in public headers as "Internal. Subject to change without notice. If you feel this function should be public, file a bug report with your use-case."?
On 07.03.24 10:09, Volker Hilsheimer wrote: >> On 4 Mar 2024, at 10:57, Marc Mutz via Development >> <development@qt-project.org> wrote: >> >> Hi, >> >> TL;DR: >> - Treat all APIs not clearly marked as private (private access, *Private >> namespace or "We mean it!" comment) as public, in particular keep SC/BC >> and deprecate before remove. >> - Avoid adding APIs that aren't clearly private or public in the future. > > […] > >> I would suggest that we treat any entity defined in a header file that >> is not in a *Private namespace or under the umbrella of an "We mean it!" >> commant, in short: anything that isn't _visible_ as non-public API _from >> the header file_, as semi-public, and apply the same rules as for public >> API: keep BC/SC, deprecate first, remove second, and adjust QUIP-6 >> accordingly. > > I think we have to acknowledge that Qt is 30 years old this year, and over > the decades the capabilities of tools, the features of the languages, and the > best practices, skills, and awareness of the people working on Qt have > changed a lot. What usually hasn’t changed is the *intent* of the class > author - even though it might be buried deep in the historical repositories. > Stating now that everything that has the technical appearance of a public API > cannot be changed, and ignoring the author’s intent, does not seem > constructive. Two users have come out and said that they think public undocumented API is fair game. If there's a difference between the expectations of users and expectations of API authors, do you _really- propose to pick the author's? > >> ¹ Off the top of my head: QString::isSimpleText(), QPen::data_ptr(), >> QDeferredDeleteEvent > > Undocumented APIs in any SDK are, IMHO, “use at your own risk”. I (or my IDE) > might find something promising in some public header, but if I have no > information about what the author of that thing is promising me, should I use > it in my code? Sometimes it’s more obvious than other times, but what on > earth is the semantics of "QString::isSimpleText”? What is that “DataPtr” > reference returned by "QPen::data_ptr”? Can anyone use > QDeferredDeleteEvent::loopLevel() in a meaningful way without studying the > event delivery code? It's not for me to prove that these functions are useful or not. It's for the changers and removers of these functions to prove that no-one uses them. But ok, let's see: - the data_ptr() could be used as a null-check (there's no QPen::isNull()) - QDeferredDeleteEvent could have been posted to an object in another thread, because there was doubt as to whether deleteLater() was actually thread-safe, but posting events is known to be (_I_ had to look that up myself just now). I have no use-case for isSimpleText(), but what, exactly, did the Qt project gain by removing it instead of deprecating it as "We have found no use for this function. If you use it, please report a bug."? The question that remains unanswered is this: Given that _we_ know how to deal with change and _we_ control the changelog, why should our _users_ be subjected to unannounced breaks of in-good-faith code? There's a time for such changes: a major release. A minor release is - IMHO - not a time for such changes. We have all the tools we need to not step on our users' toes here, while still pre-programming for a smooth transition for us come Qt 7. If we decide to nevertheless do these changes before Qt 7, what, exactly, does that say about our priorities? >> At the same time, we should make sure that we don't accept such >> semi-public APIs going forward anymore. > > I do agree that we shouldn’t add more such stuff, at least not without making > it visible in the header, ideally in a way that makes it visible to code > completion (if that’s possible at all). > > For stuff that is there today (e.g. various container types’ “isSharedWith” > public member function, usually documented explicitly as \internal, and often > used in other modules or in tests so not really practical to do it any other > way), should we add a "// internal” comment in the header file? Why is isSharedWith() \internal in the first place? The containers are publicly documented to be implicitly shared, so the likes of isShared(), isSharedWith() and detach() should be public documented functions, IMHO. For new \internal API, something like _internal as part of the function name is probably a good idea. Then qdoc could automatically skip them. Oh, that gives me an idea: QDoc could take \internal members of a documented class/namespace (or defined in a public header) and explicitly document them as "Internal, subject to change w/o notice. If you feel this entity should be public API, report a bug <link>.". That has the advantage that it solves the problem for all existing functions, in one fell swoop, even for past Qt versions. Give it an LTS cycle to sink in, and then we grant ourselves the freedom to act on it again. Even so: deprecating instead of removing should remain the gold standard. But QPen::data_ptr() would have been covered by this, at least. Thanks, Marc -- Marc Mutz <marc.m...@qt.io> Principal Software Engineer The Qt Company Erich-Thilo-Str. 10 12489 Berlin, Germany www.qt.io Geschäftsführer: Mika Pälsi, Juha Varelius, Jouni Lintunen Sitz der Gesellschaft: Berlin, Registergericht: Amtsgericht Charlottenburg, HRB 144331 B -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
