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

CODECONVENTIONS.md: added documentation on documenting code. #1322

Closed
wants to merge 2 commits into from
Closed

CODECONVENTIONS.md: added documentation on documenting code. #1322

wants to merge 2 commits into from

Conversation

vitiral
Copy link
Contributor

@vitiral vitiral commented Jun 13, 2015

completes #1235


// This function will always recurse indefinately and is only used to show
Copy link
Contributor

Choose a reason for hiding this comment

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

^ should be indefinitely

@pfalcon
Copy link
Contributor

pfalcon commented Jun 13, 2015

Looks good to me.

@danicampora
Copy link
Member

Isn't this obsolete now:

Header files:
Try to stick to the Plan 9 header style, where header files do not include other header files.
Don't protect a header file from multiple inclusion with #if directives.

?

@vitiral
Copy link
Contributor Author

vitiral commented Jun 13, 2015

Thanks for the spelling suggestions @pfalcon. I did an --amend comment and pushed with -f like @dhylands suggested in my other commit. Was that the correct way to do things?

@danicampora it does seem to be outdated from other header files I've looked at, but I don't know. I could remove it in this PR if people want it removed.

@danicampora
Copy link
Member

Thanks for the spelling suggestions @pfalcon. I did an --amend comment and pushed with -f like @dhylands suggested in my other commit. Was that the correct way to do things?

That's how I do it ;-)

@danicampora it does seem to be outdated from other header files I've looked at, but I don't know. I could remove it in this PR if people want it removed.

Let's see what others think.

- In general, there should not be lengthy or unnecessary comments. If something
may be confusing to new contributors or people unfamiliar with computer
science, it is fine to use a few key words and a link to a wikipedia article
or a wiki page.
Copy link
Member

Choose a reason for hiding this comment

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

I hate to be picky on such a trivial thing, but the philosophy of what you've written here goes against itself: each point is an embellishment of the previous one! It would be enough to say "be concise". If you want to say more than that then "be concise and only write comments for things that are not obvious" would be pretty good.

@dpgeorge
Copy link
Member

Yes the stuff about Plan 9 headers is now obsolete, but it should be removed in a separate patch.

@pfalcon
Copy link
Contributor

pfalcon commented Jun 18, 2015

Header conventions are addressed in 2474c2a

@dpgeorge
Copy link
Member

Merged in f64e080.

@dpgeorge dpgeorge closed this Jun 25, 2015
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.

None yet

5 participants