Closed hliberacki closed 3 years ago
[...] I can approve it, but I will wait some time for others to comment on this. I am sure that @slci was the one who first proposed it in the Gitter chat, and the @insooth was against it.
I'm totally for 120 lines. ;)
120 is a bit too much, that's 50% more text for eye jumping... 100 shall be enough, and I believe that there will be individual cases where the 80 is exceeded anyway.
So @hliberacki @slci are you ok with it? Can we go with a 100 char column limit?
I think 100 can be a good compromise
Yeah, 100 is always better than 80. I agree that 120 is far more than typical human can make an eye-fix for reading. But this is only valid when we want to read the code as novel. I don't see a problem with formatted code though. I tried to find out a way of formatting in-line Doxygen comments (those are the most problematic IMO) but there seems to be no rules to make it nice. I'd love to have different limit for code and for comments, but for now we can make a compromise. What about 113? Nice, prime number ;)
Ok, so it seems that we have an agreement. @hliberacki, please change the Issue and the PR names to be adequate to this change. Moreover, it looks like the spdlog version changed, and now causing a crash. Could you also handle it?
Ok I will handle it
Merging #30 into master will not change coverage. The diff coverage is
n/a
.
@@ Coverage Diff @@
## master #30 +/- ##
=======================================
Coverage 25.00% 25.00%
=======================================
Files 2 2
Lines 4 4
=======================================
Hits 1 1
Misses 3 3
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 8d1779d...58b4d91. Read the comment docs.
29