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

ICU-13810 Doxygen warning cleanup. #29

Merged
merged 1 commit into from
Aug 1, 2018

Conversation

aheninger
Copy link
Contributor

Summary of types of changes to API documentation to remove Doxygen warnings:

Doxyfile.in: upgraded using the tooling provided with the latest Doxygen, resulting in file reformatting, removal of obsolete items, and addition of new items. Doxygen was warning that the old config file was out-of-date.

U_NOEXCEPT, U_OVERRIDE, U_FINAL definitions had complicated conditionals based on platform and C++ language level that confused Doxygen. Since we now require a reasonably conforming C++ 11, I deleted many of these conditionals.

Warnings for missing docs on forward declarations, @internal classes, and similar: Use Doxygen \cond ... \endcond to skip the declarations.

html vs. Markdown style docs. Some of the html markup, probably originating from JavaDoc, was not working with Doxygen. Changed to markdown. Sometimes changed other adjacent markup also, to avoid a confusing mixture of styles. (Markdown seems to be Doxygen's preferred style, and definitely makes for much cleaner header files, which is also important.)

Fixed various typos, missing docs, etc.

NumberFormatter errors. Not fixed. These will be done by Shane.

@aheninger aheninger requested a review from gnrunge July 31, 2018 01:41
@CLAassistant
Copy link

CLAassistant commented Jul 31, 2018

CLA assistant check
All committers have signed the CLA.

# define U_NOEXCEPT
#elif U_CPLUSPLUS_VERSION >= 11 || __has_feature(cxx_noexcept) || __has_extension(cxx_noexcept) \
|| (defined(_MSC_VER) && _MSC_VER >= 1900) /* Visual Studio 2015 */
# define U_NOEXCEPT noexcept
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think VS2015 is still technically a supported platform? Though perhaps this could be reconsidered?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think VS 2015 is OK. It has MSC_VER==1900, so the original condition is met and U_NOEXCEPT is defined to be noexcept, which is what the new version does unconditionally.

Where things could differ is if _HAS_EXCEPTIONS == 0. I'm hoping it will be OK because we are restricting ourselves to compilers new enough to recognize noexcept as a keyword, which shouldn't depend on _HAS_EXCEPTIONS.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good point. Yes, the define would be unchanged for VS2015. (Thanks).

I believe the macro _HAS_EXCEPTIONS is really only for the STL library though (possibly just the MSVC STL), so I don't think having it set or not would affect the core language itself.

@aheninger aheninger merged commit 55c3309 into unicode-org:master Aug 1, 2018
Copy link
Member

@srl295 srl295 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

post review lgtm

@aheninger aheninger deleted the ICU-13810-SQ branch August 10, 2018 00:25
sffc referenced this pull request in sffc/icu Sep 26, 2018
sffc referenced this pull request in sffc/icu Sep 26, 2018
sffc referenced this pull request in sffc/icu Sep 27, 2018
sffc referenced this pull request in sffc/icu Sep 27, 2018
sffc referenced this pull request in sffc/icu Sep 27, 2018
sffc referenced this pull request in sffc/icu Sep 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants