FreeCAD / DevelopersHandbook

A handbook about FreeCAD development
47 stars 30 forks source link

Add Best Practices chapter #107

Open kadet1090 opened 1 week ago

kadet1090 commented 1 week ago

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!

3x380V commented 1 week ago

I'm happy someone is looking int this. Just a minor note: Diffs are very hard to read and also introduced few whitespace changes pre-commit is not happy about. Original is a formatted text document with line breaks up to 80 characters. The changes are somewhat autoformated text flow which makes diff too noisy. Could that be changed (if you really insist on this modern way of text formatting) so that reformat without changes goes first and changes follows in separate commits?

hyarion commented 1 week ago

I'm happy someone is looking int this. Just a minor note: Diffs are very hard to read and also introduced few whitespace changes pre-commit is not happy about. Original is a formatted text document with line breaks up to 80 characters. The changes are somewhat autoformated text flow which makes diff too noisy.

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.

Could that be changed (if you really insist on this modern way of text formatting) so that reformat without changes goes first and changes follows in separate commits?

Github can hide some white space changes, here's a link which should do it automatically for the commit: https://github.com/FreeCAD/DevelopersHandbook/pull/107/commits/a08f820f1b83ec513100dd5b04ceff0786fa6862?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?

3x380V commented 1 week ago

@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?

hyarion commented 1 week ago

It is a third party preview extension that I use in vscode. It is probably just a slightly different dialect of markdown.

3x380V commented 1 week ago

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.