Skip to content

Conversation

@Yida-Lin
Copy link
Member

#25

Standardize code style according to https://google.github.io/styleguide/cppguide.html.

Didn't have time to do functionality change, only style cleanup for now.

Note that some method name changes require the code changes in SigViewer as well.

@Yida-Lin Yida-Lin requested review from cbrnr and tstenner May 17, 2020 09:40
@Yida-Lin Yida-Lin self-assigned this May 17, 2020
@Yida-Lin Yida-Lin changed the title 25: Refactor the code according to the Google C++ Style Guide Issue-25: Refactor the code according to the Google C++ Style Guide May 17, 2020
@Yida-Lin Yida-Lin changed the title Issue-25: Refactor the code according to the Google C++ Style Guide 25: Refactor the code according to the Google C++ Style Guide May 17, 2020
@Yida-Lin Yida-Lin changed the title 25: Refactor the code according to the Google C++ Style Guide Issue-25: Refactor the code according to the Google C++ Style Guide May 17, 2020
@cbrnr
Copy link
Collaborator

cbrnr commented May 17, 2020

Where does the style guide say that function names must be camel case?

@Yida-Lin
Copy link
Member Author

Yida-Lin commented May 17, 2020

@cbrnr I agree it looks a bit odd; it's not even camel case, it's Pascal case. It says it here: https://google.github.io/styleguide/cppguide.html#Function_Names

The C++ standard library does not adopt this style though http://www.cplusplus.com/reference/algorithm/

@Yida-Lin Yida-Lin marked this pull request as draft May 17, 2020 10:47
@cbrnr
Copy link
Collaborator

cbrnr commented May 17, 2020

It's your decision. Coming mainly from Python this looks really weird.

@Yida-Lin
Copy link
Member Author

@cbrnr By looking at this thread, looks like the snake_case() makes the most sense to me.

@cbrnr
Copy link
Collaborator

cbrnr commented May 17, 2020

Yes, I'd go with the conventions used in the standard lib (at least for this case).

@Yida-Lin Yida-Lin marked this pull request as ready for review May 17, 2020 21:51
@Yida-Lin
Copy link
Member Author

@cbrnr @tstenner think I cleaned up most stylistic things; most files changes in this PR are documentation changes auto generated by Doxygen, which you can ignore. Only xdf.h, xdf.cpp, and README.md are relevant.

@tstenner
Copy link
Contributor

Personally, I prefer the LLVM over the Google style guide, but any style is an improvement over an inconsistent or undocumented style.

Two things: Having the compiled doxygen documentation in the repository adds a lot of noise to the commits. For liblsl, we use readthedocs to host the doxygen output so only the doxygen configuration files are in the repository. One other option is to include the doxygen output in the released archives so anyone downloading it has the html documentation.

The other things I'm missing is a clang-format file (labstreaminglayer example, interactive clang-format generator so that Visual Studio / QtCreator can automatically reformat the code according to the style guide.

@cbrnr cbrnr changed the title Issue-25: Refactor the code according to the Google C++ Style Guide Refactor the code according to the Google C++ Style Guide May 18, 2020
@Yida-Lin
Copy link
Member Author

Yida-Lin commented May 21, 2020

@tstenner Thank you for your input, sounds great. I will look into these over the weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants