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

DOXYGEN: updates in header files 1 #2467

Open
wants to merge 1 commit into
base: master
from

Conversation

@b-gent
Copy link

b-gent commented Sep 15, 2020

Add and improve doxygen blocks in the header file.

Just opening a PR with a single file to get some feedback from you if this is the way to proceed.

Output is available on https://b-gent.github.io/PR2467/group__common.html
@sev- @Mataniko

/**
* Set a platform and application ID as active domain.
*
* @param[in] platform Achievements platform.

This comment has been minimized.

@b-gent

b-gent Sep 15, 2020 Author

The big question is whether we should include documentation for all parameters in every method. I think this is a good practice but it definitely adds a lot of doxygen noise to the header files. If parameters were not documented, most of these class members could be documented with //!< in the same line, instead of adding a preceding doxygen block.

@b-gent b-gent force-pushed the b-gent:improve_doxygen_doc branch from f7a9ace to c9f0c32 Sep 16, 2020
bool unsetActiveDomain();
bool isReady() { return _iniFile != nullptr; }
bool unsetActiveDomain(); //!< Unset the current active domain.
bool isReady() { return _iniFile != nullptr; } //!< Set the domain as ready.

This comment has been minimized.

@b-gent

b-gent Sep 16, 2020 Author

Not sure if this is a correct description.

This comment has been minimized.

@sev-

sev- Sep 17, 2020 Member

Checks if the domain is ready.

@b-gent b-gent force-pushed the b-gent:improve_doxygen_doc branch 5 times, most recently from 5b823e5 to b778eca Sep 16, 2020
@b-gent b-gent changed the title DOXYGEN: updates in achievements.h DOXYGEN: updates in header files 1 Sep 17, 2020
@b-gent b-gent force-pushed the b-gent:improve_doxygen_doc branch from b778eca to bdae444 Sep 17, 2020
@sev-
sev- approved these changes Sep 17, 2020
Copy link
Member

sev- left a comment

Looks good. Yes, non-trivial public methods and their parameters would preferably need to be documented.

Trivial ones are, well, trivial :)

bool unsetActiveDomain();
bool isReady() { return _iniFile != nullptr; }
bool unsetActiveDomain(); //!< Unset the current active domain.
bool isReady() { return _iniFile != nullptr; } //!< Set the domain as ready.

This comment has been minimized.

@sev-

sev- Sep 17, 2020 Member

Checks if the domain is ready.

* "game" (case insensitive) in the path "directory" first and search through all
* of the matches for "itedata" (case insensitive too).
*
* Note that it will add *all* matches found!
* Note that it will add all matches found!

This comment has been minimized.

@sev-

sev- Sep 17, 2020 Member

This supposed to stress the word "all", by putting it in italic.

This comment has been minimized.

@b-gent

b-gent Sep 17, 2020 Author

I can revert this change if you want but honestly, whether we stress it or not, this sentence pretty much doesn't say anything else and it's impossible to miss the 'all'.

* It is also possible to add sub directories of sub directories (of any depth) with this function.
* The path seperator for this case is SLASH for *all* systems.
* It is also possible to add subdirectories of subdirectories (of any depth) with this function.
* The path seperator for this case is SLASH for all systems.

This comment has been minimized.

@sev-

sev- Sep 17, 2020 Member

Same thing here.

@b-gent
Copy link
Author

b-gent commented Sep 17, 2020

I have two more files changed locally on this branch which I will push today so please wait a bit with approving/merging.

Also, as you can see in the output, for example here:
https://b-gent.github.io/PR2467/group__common__alg.html
there are a few undocumented structures here. Easy to spot as they are in black, plain text instead of blue, hyperlinked.

There are not so many in the 3 first files but I am now working with arrays.h and about half of the functions there is not described:
https://b-gent.github.io/PR2467/class_common_1_1_array.html

@sev- Do you think we have the capability for developers to provide me with these descriptions? Even if not for all structures, we need someone to identify which are the more important ones that need documentation and which we can skip (at least for now).

It seems to me providing a description for everything will be barely possible. A file like arrays.h misses documentation for about 30 structures. And there are 80 headers in the common folder alone.

@b-gent b-gent force-pushed the b-gent:improve_doxygen_doc branch from bdae444 to 8297578 Sep 18, 2020
/**
* A template implementing a bit stream for different data memory layouts.
*
* Such a bit stream reads valueBits-wide values from the data stream and
* gives access to their bits, one at a time.
*
* For example, a bit stream with the layout parameters 32, true, false
* for valueBits, isLE and isMSB2LSB, reads 32bit little-endian values
* for valueBits, isLE and isMSB2LSB, reads 32-bit little-endian values
* from the data stream and hands out the bits in the order of LSB to MSB.
*/
template<class STREAM, int valueBits, bool isLE, bool MSB2LSB>

This comment has been minimized.

@b-gent

b-gent Sep 18, 2020 Author

This class is nicely documented, only misses docs for two small structures in lines 277 and 281.
https://b-gent.github.io/PR2467/class_common_1_1_bit_stream_impl.html

In contrast, BitStreamMemoryStream lacks docs for all functions:
https://b-gent.github.io/PR2467/class_common_1_1_bit_stream_memory_stream.html

@b-gent b-gent marked this pull request as ready for review Sep 18, 2020
@b-gent
Copy link
Author

b-gent commented Sep 18, 2020

I pushed two more files, I think I won't add any more to this PR and open another one instead. Splitting this into more PRs will make it easier to manage and review.

The output on github.io is also updated.

@sev- please re-check and address my comments.

@b-gent b-gent force-pushed the b-gent:improve_doxygen_doc branch from 8297578 to 5e01f62 Sep 18, 2020
Add and improve doxygen blocks in header files:

- achievements.h
- algorithm.h
- archive.h
- array.h
- bitstream.h
@b-gent b-gent force-pushed the b-gent:improve_doxygen_doc branch from 5e01f62 to 2ab3759 Sep 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.