-
Notifications
You must be signed in to change notification settings - Fork 22
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
[feature] Enable or disable doxygen during build phase #37
base: master
Are you sure you want to change the base?
[feature] Enable or disable doxygen during build phase #37
Conversation
A new option is added to cmake 'ENABLE_BUILDING_DOCS', set to 'ON' by default. Using `ccmake` this variable can be set to OFF which will allow building SRecord executable only.
I though about prefixing every switch with |
As the CMake files are brand new, I want to ask the question, if the default ON is worth to keep. |
Will make this change, I did a survey of some other projects (primarily LLVM & Clang) and will use the naming convention |
I am for disabling documentation generation during debug build. I find for projects I work with I use debug when iterating a design and use release when finished and need the documentation.
I think it would be within cope to add a release and debug build option to this PR. I would base the builds off Professional CMAKE, Chapter 13.1.1. Single Configuration Generators. This would then allow having support for multiple build configurations in a separate directory. |
A question: do you really want to disable the documentation, or is it enough to remove the |
That's a common, but an old naming practice of CMake 2.x. For me it's like this: |
Changed cmake variable for enabling documentation to `SRecord_ENABLE_DOCS` and set default behavior to `OFF`. Also modified how document generation is determined. If `SRecord_ENABLE_DOCS` is true, then documentation will always be generated, otherwise documentation will only be generated for a release build. If documentation is to be generated and doxygen not found, then a fatal error will be generated.
Latest commit addresses this the behavior change. I created an internal variable in CMake, and am looking to see if this is the right way to accomplish what we want. Open to suggestions. |
SRecord has always been the standard capitalisation so I agree. However, I can't ignore that it does look a little clunky. Should we consider all-caps to give SRECORD_ENABLE_XXX style variables? Is this more in keeping with other projects. I'd need to check if this has any side effects such as generated filenames but still. Thoughts gents? |
Another option, which I feel is a nice middle ground, would be the following:
Here all options for SRecord would be based upon
|
I try to explain my rationale, which might be unnecessarily complex: But it's also library. And I see two ways to use it nowadays (AFAIK):
As the module file in 1. needs to be generated by us, it can use any prefix, even different from the project. In fact, I believe we may not need to add any extra variables there on our own. Perhaps some like: gcrypt was used for the compiled library or not. For 2. we definitely need a prefix. And for consistency with 1. I suggest the same naming. You might decide on naming later, when 1. or 2. is really supported. I did the suggestion now, to avoid naming changes. Until now, only ArchLinux seems to use the new version (see Repoolgy). I'm no user if CMake aware libraries myself (on my job), but I can ask those questions on CMake's discourse and hope for some experts to answer. |
A new option,
ENABLE_BUILDING_DOCS
, is added tocmake
and defaultsON
to preserve legacy behavior. Usingccmake
,ENABLE_BUILDING_DOCS
can be toggledON
orOFF
allowing document generation only when needed.Issue accomplishes one of the tasks in Issue #29.