A subset of LibreOffice source headers defines different parts of LibreOffice published API. It would be good to have a standardized comment after the license statement of those headers unambiguously mentioning that fact, so that a developer could see immediately if this is part of published API or not. This is required to clearly see what can be changed and what can't, or needs special treatment, including public announcement of deprecation, using conditional compilation for internal vs external usage, etc. The task is: come up with a suggested comment and document it in README.md; identify the published subset (communicating with developers in IRC and mailing list); add the comment to all files in the subset.
(The relevant C/C++ include files are those that are provided by the LO SDK. See also odk/CustomTarget_allheaders.mk, which assembles a list of those for different, testing purposes.)
Hello, could you help me how to find these files that need the comments? I would like to help in this problem. 1- How can I have access the files provided by the LO SDK? 2- How I can use the odk/CustomTarget_allheaders.mk ? I have already compiled the full package with success. Thanks for the attention.
Regarding comment 1, run make CustomTarget_odk/allheaders Then go look at workdir/CustomTarget/odk/allheaders/allheaders.hxx
I would like to work on this issue. I'll soon come with possible solution and report here.
Created attachment 169147 [details] Proposal for unambiguous comment to each published API This .odt file contains the proposal for solving the bug. Please suggest modifications to optimize the proposed solution. Thanks, Srijita
(In reply to Srijita from comment #5) > This .odt file contains the proposal for solving the bug. > Please suggest modifications to optimize the proposed solution. The proposal is very detailed and nice. Thanks for working on this! > standardized unambiguous comment after the license statement if this is part of published API or not Please note that we only want the "if this is part of published API", and we don't want the "or not" - i.e., we don't expect all headers that are *not* part of published API to have the comment. It seems that you know this (from your proposal details), but I thought I mention this for completeness. > /* > * This file is part of LibreOffice published API under the "ABC” subset. > * This filecannot be changed due to “XYZ” reason. This file needs XYZ > * special requirements. > * > * This file is bound to deprecate and will no longer work from date > * “XX-XX-XX”. For more information regarding suggested workarounds > * and removal dates visit https://XXXXXX. > / Very nice formal comment :-) But let's do something less formal. "This file is part of LibreOffice published API" is fine; but let's avoid the "subset" part. We do not block changing the headers, as long as this is either backward-compatible, or related to removal of old deprecated things. So the "This file cannot be changed due to “XYZ” reason" should also be omitted. If we deprecate something, this should be a separate comment (not a part of "this is a published API" comment), and needs a separate section/discussion in .md. Also it needs to be simplified, too: no need to mention specific dates, and usually we do not set some detailed milestones when deprecate things (something gets deprecated, then after some time a separate decision is considered to remove it, and maybe the deprecation gets reverted, or removal gets postponed ...) - so just something like "This header is deprecated since LibreOffice 7.1" is IMO OK. I suggest you to create a gerrit change with your proposal implemented as a patch to README.md. Then it would be easier to discuss this right there. After this is agreed upon on the README.md level, and it's merged, the next step would be to add the relevant comments to the actual headers. Thanks again!
Thanks to feedback from Stephan, I now realize that instead of README.md, putting the discussed comments into TEMPLATE.SORUCECODE.HEADER is a better choice.
Thanks for the positive feedback :). I'll take everything you said into consideration and submit a patch soon. Regards, Srijita
I have created a gerrit change as suggested. Link: https://gerrit.libreoffice.org/c/core/+/109945 Kindly review the patch. Thanks, Srijita
msrijita18 committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/e58d2561f93271879252c2d0eda6cf1a08ef9848 tdf#130978 Add a comment to each published API It will be available in 7.2.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
The next step as told by Mike is to change the actual headers accordingly. I'm thinking of creating an excel sheet where the developers can mention if published or not , specification, public deprecation date , etc and circulate it in mail list and IRC. Any suggestions?
(In reply to Srijita from comment #11) No, please just perform the necessary steps to find out which headers are published (comment 1), and prepare a patch. If needed, all discussion would happen on gerrit. Thanks!
OK, I'll submit a patch soon.
(In reply to Mike Kaganski from comment #12) > (In reply to Srijita from comment #11) > > No, please just perform the necessary steps to find out which headers are > published (comment 1), and prepare a patch. If needed, all discussion would > happen on gerrit. Thanks! Hello, I have some queries: 1- In the instdir/sdk/include/com/sun/star/uno there are five headers, should the following comment be added below the license part in a one patch? (or separate patches)? (For eg: In Any.h) /* * This file is part of LibreOffice published API. */ 2- Should any additional info be added? (like public deprecation, special treatment) 3- How can I know the different specifications to unique to different headers? Thanks, Srijita
(In reply to Srijita from comment #14) > 1- In the instdir/sdk/include/com/sun/star/uno there are five headers, > should the > following comment be added below the license part in a one patch? (or > separate patches)? It's reasonable to process related files in one patch; I'd say that ~20 files in a single directory (with subdirectories possibly) in a single patch is fine. > 2- Should any additional info be added? (like public deprecation, special > treatment) As we discussed in the gerrit change, this is only for the "part of LibreOffice published API", and if needed, other info would be under separate issue. Just limit this to the agreed wording. > 3- How can I know the different specifications to unique to different > headers? Not quite understand this. Is this related to #2?
(In reply to Mike Kaganski from comment #15) > > 3- How can I know the different specifications to unique to different > > headers? > > Not quite understand this. Is this related to #2? As already mentioned that we have to limit the wording, then we actually don't need any extra comment regarding deprecation, so my 3- question becomes invalid :-) Then I'll only add one comment as decided. Thanks!
After half an hour of trying to git add files of /instdir I realised it can be edited as it is part of .gitignore . How should I proceed ahead? Thanks, Srijita
(In reply to Srijita from comment #17) Files in instdir are generated from some IDLs in the core. It doesn't make sense to have the notice in auto-generated files. Only add the notice to the files in the version control - those that are in include/, and those IDLs that are used to generate the files you mentioned.
Right, thanks!
I have created the gerrit change. Link: https://gerrit.libreoffice.org/c/core/+/110018 Kindly review the patch. Thanks, Srijita
(In reply to Srijita from comment #17) > After half an hour of trying to git add files of /instdir I realised it *can not" be edited as it is part .gitignore (mentioning in case of some misunderstanding) Sorry for the typo!
msrijita18 committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/83f0ec0ac179feb188fc0c9184d5c57c1870fde4 tdf#130978 Add a comment to each published API It will be available in 7.2.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
msrijita18 committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/b1fb6338cf483a1fd8c4a16746c8f7c3af8cba9e tdf#130978 Added comment to all published API It will be available in 7.2.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Comment added in all headers part of published API.