Add Best Practices chapter #107
Conversation
Converted from a document originally posted on FreeCAD forum: https://forum.freecad.org/viewtopic.php?t=89913
codeformatting/c++practices.md
Outdated
| ```cpp | ||
| using PrefixSpec = struct { | ||
| char prefix; | ||
| unsigned long long factor; | ||
| }; | ||
|
|
||
| auto prefix = spec->first; | ||
| static constexpr std::array<PrefixSpec, 7> sortedPrefixes { | ||
| {{'E', 1ULL << 60}, // 1 << 60 = 2^60 | ||
| {'P', 1ULL << 50}, | ||
| {'T', 1ULL << 40}, | ||
| {'G', 1ULL << 30}, | ||
| {'M', 1ULL << 20}, // 1 << 20 = 2^20 = 1024 | ||
| {'k', 1ULL << 10}, // 1 << 10 = 2^10 = 1024 | ||
| {'\0', 0}}}; | ||
|
|
||
| const auto res = std::find_if(prefixes, [&](const auto& spec) { | ||
| return spec.factor <= size; | ||
| }); | ||
|
|
||
| // Add one digit after the decimal place for all prefixed sizes | ||
| return res->factor | ||
| ? fmt::format("{:.1f} {}B", static_cast<double>(size) / res->factor, res->prefix) | ||
| : fmt::format("{} B", size); | ||
| ``` |
There was a problem hiding this comment.
This example should be replaced with outcome from this pr: FreeCAD/FreeCAD#17817
|
I'm happy someone is looking int this. Just a minor note: |
I'm sorry, the markdown editor I use in vscode has a preview that break lines on newlines, so I removed them to avoid some lines breaking in odd places. It didn't occur to me that it might be a bug in my tool. I thought the line endings was a mistake.
Github can hide some white space changes, here's a link which should do it automatically for the commit: a08f820?diff=unified&w=1 Does that work for you @3x380V or do you want me to reformat the changes to make it as close as possible? |
|
@hyarion, I can deal with that diff using various tips and tricks to make it more readable, but it is rather unexpected from editor to randomly break lines in C++ or Python code, so why is that default here? Why do we have ~500 character long lines in this document? It breaks each and every line based tool. And both patch and diff are line based tools. Could be VSCode told just not trying to be smart? |
|
It is a third party preview extension that I use in vscode. It is probably just a slightly different dialect of markdown. |
|
Ok. Let me put it this way: this chapter had no formatting, it is just a conversion of PDF uploaded to the forum, which itself was created in wisiwig editor. So formatting is mine (and done the way I'm used to format such documents). If you are familiar with whatever extension you are using, just reformat that commit the way you like and which is for most people here easy to work with. I'll probably close original PR as it served its purpose already. |
kadet1090
force-pushed
the
code-review-guidelines-design
branch
3 times, most recently
from
d94cf5e
to
4a81308
Compare
This change aims to improve language of the C++ practices so it is more welcoming for newcomers. Wording is different, but the ideals presented were kept as they were good.
This adds a guide aimed more at reviewers that takes the guide and tries to extract from it steps that are easy to follow and check. This commit also reorganizes the structure so everything lives in a new bestpractices folder. Co-Authored-By: Benjamin Nauck <[email protected]>
kadet1090
force-pushed
the
code-review-guidelines-design
branch
from
4a81308
to
b29e281
Compare
This is followup for the #101 (Add chapter with C++ recomended practices) PR with aim to make the proposed document more welcoming to new users and easier to enforce by providing set of quite concrete rules that should be possible to check on code review. This is joint effort of @hyarion, me and authors of the original PR: worser (forum) and @3x380V as this one is based on it. We are doing it as separate PR as this is easiest way to propose changes, but the original commits were left intact.
Instead of putting it inside code formatting, content is placed in its own chapter so it is hopefully easier to find.
By no means this is a finished piece of work - I think of it as the start point to which we all should contribute and extend it together. Python section is still missing so if we have some people that are good with this language - we would appreciate your help!