Skip to content

Codestyle rtlib#32

Merged
sjanssen2 merged 10 commits intomasterfrom
codestyle_rtlib
Jun 18, 2020
Merged

Codestyle rtlib#32
sjanssen2 merged 10 commits intomasterfrom
codestyle_rtlib

Conversation

@sjanssen2
Copy link
Member

@sjanssen2 sjanssen2 commented Jun 11, 2020

Similar to #31 for the sub-directory rtlib/
Bringing the number of warnings down from 4805 to "only" 606.
Let us merge #31 first!

@@ -1,5 +1,5 @@
#ifndef HASH_STATS
#define HASH_STATS
#ifndef RTLIB_HASH_STATS_HH_
Copy link
Member

Choose a reason for hiding this comment

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

Why is it considered a better style to add underscores at the end?

Copy link
Member Author

Choose a reason for hiding this comment

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

honestly: I don't know and I cannot find anything about it in Google's styleguide: https://google.github.io/styleguide/cppguide.html#Header_Files
But hey, as long as it doesn't break anything we can comply to this rule

rtlib/table.hh Outdated
Comment on lines +454 to +455
// std::cerr << "i " << i << " j " << j << " a " <<
// a << " b " << b << std::endl;
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if I like this style, personally. To split a single-line comment into two lines because - I assume - the comment is considered too long? But that's just personal preference.

Copy link
Member Author

Choose a reason for hiding this comment

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

I personally agree, but for (historic) reasons, source code lines should never exceed 80 characters. I guess this is due to the fact that early day's terminals were exactly 80 characters wide and if you think of tools like diff it makes sense to have finite numbers of chars in a line. I would prefer a larger cut-off, but if the world picks 80 than let's go with it.

#ifndef BIGINT_HH
#define BIGINT_HH
#ifndef RTLIB_BIGINT_HH_
#define RTLIB_BIGINT_HH_
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a cpplint thing or our own new standard of naming constants?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wouldn't have touched it if not cpplint is forcing me to do. It is their style.

@sjanssen2 sjanssen2 merged commit 772e42e into master Jun 18, 2020
@sjanssen2 sjanssen2 deleted the codestyle_rtlib branch December 3, 2020 13:15
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.

3 participants