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

Fixed doxygen comments, focused on file headers and group definitions #251

Merged
merged 4 commits into from
Dec 16, 2013

Conversation

haukepetersen
Copy link
Contributor

Hi @ALL,

as I am new to RIOT I will give my first shot at putting a pull request together: while looking through the code I tried to clean up some of the doxygen comments in RIOT, mainly focusing on bringing the doxygen module structure in order. I tried to unify the page header comments to the following style and order:

  1. Copyright
  2. Group assignment
  3. File information
  4. Authors

I tried to use the following blueprint for each (.h) file:

/*
 * Copyright (C) 2013 Freie Universität Berlin
 *
 * This file subject to the terms and conditions of the GNU Lesser General
 * Public License. See the file LICENSE in the top level directory for more
 * details.
 */

/**
 * @addtogroup  core_util
 * @{
 */

/**
 * @file         filename.h
 * @brief       Description whats in that file
 * @author    Institution...
 * @author    Author xyz <[email protected]>
 */

// beautiful code

/** @} */

and for .c files:

/*
 * Copyright (C) 2013 Freie Universität Berlin
 *
 * This file subject to the terms and conditions of the GNU Lesser General
 * Public License. See the file LICENSE in the top level directory for more
 * details.
 */

/**
 * @addtogroup  core_util
 * @{
 * @file         filename.c
 * @brief       Short description of whats in here
 * 
 * If needed: long description of the file
 *
 * @author    Institution...
 * @author     Author xyz <[email protected]>
 * @}
 */

// here comes the code

For group definitions that are not clearly traceable to a specific file I inserted doc.txt files into the module folders (e.g. RIOT/core/doc.txt, RIOT/sys/net/doc.txt, ...). The location and naming of these files is surely something we should talk about...

@kaspar030
Copy link
Contributor

Welcome on board! ;)

A quick look shows there are some whitespace issues. AFAIR we've decided to use 4 spaces instead of tabs.

@OlegHahm
Copy link
Member

@kaspar030, you're right: https://github.com/RIOT-OS/RIOT/wiki/Coding-conventions#indentation-and-braces but I don't see where this PR breaks this (though I might have overlooked it).

Thanks for the work, @haukepetersen.

@kaspar030
Copy link
Contributor

At least in mutex.h and msg.h some doxygen comments use e.g., tabs for @author and spaces for the rest, or vice-versa, in the same file. Can see that in github only whith a sharp eye.

@haukepetersen: looks like your last commit is astyling a lot more than the changes you've made before. ;)

@haukepetersen
Copy link
Contributor Author

Oh, now I see it too - damnit. How should we proceed, should I revert the last commit and try to fix the tab issue maually only to the parts I changed or should we keep the other astyle fixes in as well (since the style in that code lines was not up to the standard anyway...)?

@OlegHahm
Copy link
Member

Auto-astyling the whole code is not a good idea. At some particular places it makes sense to interpret the coding conventions a bit more flexible (e.g. astyle would make the lcd_font array in https://github.com/RIOT-OS/boards/blob/master/chronos/drivers/display1.c mostly unreadable).

Hence, I suggest to remove the last commit, ran astyle again, but commit only the files you've touched and review carefully if the astyling "breaks" anything.

@haukepetersen
Copy link
Contributor Author

Yes, sounds very reasonable, will do!

@kaspar030
Copy link
Contributor

coloured git-diff and searching for tab helps in manually fixing these things

@OlegHahm
Copy link
Member

Needs rebasing.

@miri64
Copy link
Member

miri64 commented Nov 3, 2013

After rebasing and fixing of the tab problem I'd like to ack this. Even used this documentation scheme in #290

@mehlis
Copy link
Contributor

mehlis commented Nov 14, 2013

@haukepetersen ping

1 similar comment
@LudwigKnuepfer
Copy link
Member

@haukepetersen ping

@miri64
Copy link
Member

miri64 commented Nov 25, 2013

@haukepetersen ping?

@LudwigKnuepfer
Copy link
Member

git co comments
git rebase master
git push -f

@miri64
Copy link
Member

miri64 commented Nov 26, 2013

git co does not exist in mainline git. Use git checkout instead ;-)

@haukepetersen
Copy link
Contributor Author

Ok, sorry guys, took a while. But with Olegs help I was able to finally rebase the old commit. I pushed a second commit on top, where I fixed the tabs and some other formating issues.

/**
* @defgroup net_sixlowpan 6LoWPAN
* @ingroup net
* @brief Riots 6LowPAN implementation
Copy link
Member

Choose a reason for hiding this comment

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

RIOT should be always capitalized.

@haukepetersen
Copy link
Contributor Author

And here are two more amendments, now the boards folder is fully integrated after is was moved. Let me know if I should group all the commits together.

@OlegHahm
Copy link
Member

Needs rebase

@haukepetersen
Copy link
Contributor Author

Rebased...

@miri64
Copy link
Member

miri64 commented Dec 16, 2013

ACK

@mehlis
Copy link
Contributor

mehlis commented Dec 16, 2013

ACK

ready for merge?

OlegHahm added a commit that referenced this pull request Dec 16, 2013
Fixed doxygen comments, focused on file headers and group definitions
@OlegHahm OlegHahm merged commit 7d9dc6b into RIOT-OS:master Dec 16, 2013
@haukepetersen haukepetersen deleted the comments branch December 16, 2013 18:48
@haukepetersen haukepetersen restored the comments branch December 16, 2013 18:48
@haukepetersen haukepetersen deleted the comments branch December 16, 2013 18:48
LudwigKnuepfer added a commit to LudwigKnuepfer/RIOT that referenced this pull request Jan 20, 2014
remove thread_measure_stack_usage from kernel_internal.h again
CW-75 added a commit to CW-75/RIOT that referenced this pull request Jul 1, 2022
docs: solved some warnings in the RC-22.06
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.

6 participants