Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate comment with the name of the input IDL file #1925

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

knutpett
Copy link
Contributor

Knowing which source IDL file was used to generated the code makes it easier to navigate in the code.

Knowing which source IDL file was used to generated the code
makes it easier to navigate in the code.
@knutpett
Copy link
Contributor Author

@iguessthislldo

@knutpett
Copy link
Contributor Author

@jwillemsen: could you have a look at my PR? For later reference, how do I add reviewers to my PRs? Is it correct to tag/mention someone, or will eventually somebody look at the list of new PRs and pick them up?

@jwillemsen
Copy link
Member

I think only project members can add reviewers.

Open source support is really best effort, no guarantees when someone will pick it up, really depends on what people are working on. See https://github.com/DOCGroup/ACE_TAO/wiki/ACE-and-TAO-Commercial-support when you really want someone to have a look, at the end also we have to pay our bills. Merging a PR will trigger a lot of bills the next night, that is for example clearly visible on the power usage of our infrastructure :-(

@knutpett
Copy link
Contributor Author

My company have earlier paid OCI for support, for new features and/or bug fixes. And we will do that again if needed.

But for the "software gardening" PRs I've contributed just recently, do you still consider reviewing them as "support"? I understand it requires effort for you to review them. But I'm contributing improvements to the product, and my company is paying my salary doing this.

@jwillemsen
Copy link
Member

My time has to be payed by someone, I always try to review/respond as much as possible on github, but the time for that is very limited. I can't book time I spend here on the account of our customers unless I can proof to them that a specific PR has a concrete benefit for them and they give me permission to work on the PR/issue on their behalf.

@jwillemsen
Copy link
Member

Wouldn't it be better to place the file it generated from in the doxygen header in such a way doxygen also creates a link to the IDL file? The IDL filename should not include any path as the output of TAO_IDL should be independent of the location where the source IDL file is, else it is hard for people to compare the generated code between versions.

@knutpett
Copy link
Contributor Author

As an example: The file TAO/tao/IFR_Client/IFR_ExtendedS.h is generated from TAO/tao/IFR_Client/IFR_Extended.pidl. I want the generated code to include a reference to IFR_Extended.pidl. We could use the doxygen @file (or \file) tag like this:

// -*- C++ -*-
/**
* @file IFR_Extended.pidl
 * Code generated by the The ACE ORB (TAO) IDL Compiler v3.0.8
 * TAO and the TAO IDL Compiler have been developed by:
 *       Center for Distributed Object Computing
 *       Washington University
 *       St. Louis, MO
 *       USA
 * and
 *       Distributed Object Computing Laboratory
 *       University of California at Irvine
 *       Irvine, CA
 *       USA
 * and
 *       Institute for Software Integrated Systems
 *       Vanderbilt University
 *       Nashville, TN
 *       USA
 *       https://www.isis.vanderbilt.edu/
 *
 * Information about TAO is available at:
 *     https://www.dre.vanderbilt.edu/~schmidt/TAO.html
 **/

@jwillemsen
Copy link
Member

Isn't @file to document the name of the current file? See https://doxygen.nl/manual/commands.html#cmdfile.

@knutpett
Copy link
Contributor Author

Yes it is. But I don't want to refer to the current file, I want to refer to the source IDL file.

@vermaete
Copy link

Could this info be under a configuration condition?
So, if not set no comments will be generated to trace and debug this info?
My, although small, issue with it is that the QA checks in Yocto will complain about an path to a temp place in the generated code.

And kind of the same for the '// TAO_IDL - Generated from' comments.

Br

@knutpett
Copy link
Contributor Author

Could this info be under a configuration condition? So, if not set no comments will be generated to trace and debug this info? My, although small, issue with it is that the QA checks in Yocto will complain about an path to a temp place in the generated code.

I'm not including the full path to the file, just the "basename", so in the example above it would just be "IFR_Extended.pidl". (This is different from the '// TAO_IDL - Generated from' comments.)

@mitza-oci
Copy link
Member

I built libTAO locally with the merge commit of this PR. For reference, the generated code looks like this:

// -*- C++ -*-
/**
 * Code generated by the The ACE ORB (TAO) IDL Compiler v3.1.1
 * TAO and the TAO IDL Compiler have been developed by:
[...]
 *
 * Information about TAO is available at:
 *     https://www.dre.vanderbilt.edu/~schmidt/TAO.html
 **/
// Generated from: BooleanSeq.pidl

Is there consensus to (or not to) merge these two comments and use the @file doxygen command?

@jwillemsen
Copy link
Member

I would prefer to have the Generate from part within the doxygen block so that it appears in the doxygen docu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants