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

Standardize Comments #4820

Closed
wants to merge 30 commits into from
Closed

Conversation

FauziAkram
Copy link
Contributor

Comments improvement, mainly, I've noticed a mix of comment styles in our chess engine code: double slashes (//) and triple slashes (///). To enhance code consistency and readability, I propose we standardize on double slashes (//) for comments for better consistency and clarity.
Non-Functional change.

@vondele
Copy link
Member

vondele commented Oct 20, 2023

I believe the two vs. three slashes has to do with the scope or visibility of the functions that are being commented (maybe somebody can explain).

I don't have particularly strong opinion of that one, it has not been particularly useful for me, but maybe others like this.

@Disservin
Copy link
Member

Disservin commented Oct 20, 2023

A somewhat more useful change for modern ide's is to move the comment above the function directly above the function without an empty line between so that ide's can properly display the description when hovered over. I think that is kinda required by all ide's (a topic/pr on it's own).

Regarding the triple slashes vs double slashes I find it rather weird and not clear on first sight, so I'm fine with enforcing double slashes throughout the code.

@vondele
Copy link
Member

vondele commented Oct 20, 2023

so let's remove the blank line between comments and functions as part of this PR as well. It makes sense to have good IDE integration. @FauziAkram can you do that?

@FauziAkram
Copy link
Contributor Author

FauziAkram commented Oct 20, 2023

@vondele Unfortunately, I'm afraid I can't do it for now.
I just hope this PR gets accepted, and then maybe in the future, I will come back to it if nobody touched it.

@vondele vondele added the to be merged Will be merged shortly label Oct 21, 2023
@vondele vondele closed this in edb4ab9 Oct 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-bench-change to be merged Will be merged shortly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants