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

Moved documentation to header files #57

Closed
wants to merge 1 commit into from
Closed

Moved documentation to header files #57

wants to merge 1 commit into from

Conversation

apfohl
Copy link

@apfohl apfohl commented Oct 3, 2019

  • Moved all function documentation to the corresponding header file
  • Reordered functions in header files to match ordering in implementation files
  • Added missing documentation
  • Leave documentation stubs for missing documentation

- Moved all function documentation to the corresponding header file
- Reordered functions in header files to match ordering in implementation files
- Added missing documentation
- Leave documentation stubs for missing documentation
@apfohl
Copy link
Author

apfohl commented Oct 3, 2019

This PR fixed #52.

Comment on lines +806 to +810
/**
* ToDo: Check where this is coming from as it has no implementation
* @param seconds
*/
extern void cgi_session_set_max_idle_time(unsigned long seconds);
Copy link
Author

Choose a reason for hiding this comment

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

I discovered that this function prototype does not have a matching implementation. Is this still needed?

@LeSpocky LeSpocky self-assigned this Oct 3, 2019
@LeSpocky LeSpocky added the doc documentation label Oct 3, 2019
@apfohl
Copy link
Author

apfohl commented Oct 11, 2019

Are there any news on this one? Is this okay? Shall I change something?

@LeSpocky
Copy link
Collaborator

It's on my ToDo-List, will review it.

@LeSpocky LeSpocky self-requested a review October 11, 2019 21:43
Copy link
Collaborator

@LeSpocky LeSpocky left a comment

Choose a reason for hiding this comment

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

I tried to review your PR, but I find it hard. The commit message reads:

  • Moved all function documentation to the corresponding header file
  • Reordered functions in header files to match ordering in implementation files
  • Added missing documentation
  • Leave documentation stubs for missing documentation

If it's not too cumbersome for you, could you split all that in different commits like this?

  • Whitespace changes in one separate commit
  • Moving things from source to header in one separate commit (without changing content)
  • Reordering in one separate commit
  • Improving things in one or more separate commits

Please also use @todo or TODO.

I like the structuring using Doxygen defgroups. 😄

If this is too much work for you, drop me a note, I'll give it another try all in one then.

Don't be confuse about the annotations below, I started reviewing, but aborted. 👼

Comment on lines -18 to +20
* HTTP status codes.
* HTTP status codes.
*
* @see http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html
* @see http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html
Copy link
Collaborator

Choose a reason for hiding this comment

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

Whitespace change only, right?

Comment on lines -74 to +76
* General purpose linked list. Actually isn't very portable because
* uses only 'name' and 'value' variables to store data. Probably, in
* a future release, this will be replaced by another type of struct.
* General purpose linked list. Actually isn't very portable because
* uses only 'name' and 'value' variables to store data. Probably, in
* a future release, this will be replaced by another type of struct.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This also looks like whitespace change only. Whitespace changes should go to separate commits.


#define E_WARNING 0
#define E_FATAL 1
#define E_CAUTION 2
#define E_INFORMATION 3
#define E_MEMORY 4

/**
* ToDo: Add documentation
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use @todo or TODO please.

@apfohl
Copy link
Author

apfohl commented Oct 21, 2019

Yes, of cause I can restructure this PR. Give me some time.

@apfohl apfohl closed this Apr 12, 2022
@apfohl apfohl deleted the documentation branch April 12, 2022 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants