eu07 / maszyna

MaSzyna Train Simulator
http://eu07.pl
Mozilla Public License 2.0
124 stars 50 forks source link

Refactor - example #24

Closed mrozigor closed 6 years ago

mrozigor commented 6 years ago

Propozycja - dodałem kilka wpisów do formatera. Zastosowałem go dla AirCoupler (i przy okazji poszło dla DynObj.cpp). Dodatkowo chciałem lekko wyczyścić (póki co tylko AirCoupler) - usunięcie komentarzy niepotrzebnych, camelCase, inne drobne zmiany. Jeżeli nie byłoby przeszkód, to zrobiłbym resztę codebase'u (dodatkowo mogę rozwiązać konflikty, jeżeli ktoś by mergeował swoje zmiany) ;)

Milek7 commented 6 years ago

Nie mówię że nie, ale dobrze by było omówić z innymi. jak clang-format wymaga tylu dodatkowych zmian, to może jest lepszy niż LLVM styl bazowy? zrobiłeś tu dużą zmianę z BreakBeforeBraces, trzeba to ustalić, a tmj pewnie chciałby włączone SpacesInParentheses i SpacesInSquareBrackets. Z tego AirCoupler to trochę mi się śmiać chce, bo wychodzi że jak pojawia się nowy deweloper to każdy refaktoryzuje go na swoją modłę ;D (wcześniej to robił carmel) Jestem za ujednoliceniem nazewnictwa, ale to dalej wychodzi tak, że każdy robi po swojemu, bo tu masz takiCase, a ostatnio raczej stosowaliśmy taki_case. Nie ma szans na głębsze zmiany w formatowaniu dopóki działamy na trochę rozbieżnych forkach z tmj, bo później to się zrobi taki syf przy mergowaniu że nawet nie chcę myśleć.

Dzięki za poruszenie istotnego tematu, ale bez konsensusu i później doprowadzenia forków do jednego stanu nic nie zrobimy.

Milek7 commented 6 years ago

Co tam w includach zmieniałeś? stdafx.h ma być na początku każdego cpp. Reszta includów w cpp chyba że jest używana w h. tutaj widzę że uh, przeniosłeś z cpp do h?

mrozigor commented 6 years ago

To dopiero początek zmian, kilka plików poprawiłem.

[EDIT] Poczytałem różne opinie, powrócę z headerami do porządku w jakim były. Pytanie tylko po co includować stdafx.h w każdym cpp? Czy każdy to używa?

Milek7 commented 6 years ago

Includy w cpp głównie ze względu na szybkość kompilacji. stdafx.h w każdym dla jednolitości, prawie w każdym jest wymagany, jest prekompilowany to nie wydłuża to kompilacji. (no i jeszcze bywa że komuś coś zabraknie z std/glm, zadowolony dopisze specyficzny .h, tyle że będzie musiał się skompilować osobno, a z stdafx jest darmowe czasowo)

Milek7 commented 6 years ago

wygląda ok. jedna rzecz: dlaczego ConstructorInitializerIndentWidth: '0', ContinuationIndentWidth: '0'? Powinno być raczej na 4.

mrozigor commented 6 years ago

Ok, zmienię te opcje. Dam znać jak wszystkie pliki poprawię, a w międzyczasie dołożę zmiany/uwagi jakie dojdą.

mrozigor commented 6 years ago

Ok, poddaję się ;) Myślę, że nie ma sensu poprawiać całego kodu, bo zmian byłoby mnóstwo. Również odpuszczę sobie wprowadzanie zmian w ostrzeżeniach/błędach kompilatora, bo część z nich powoduje złe działanie aplikacji. Chyba najlepszym rozwiązaniem jest formatowanie kodu przez osobę, która nad nim pracuje.

Umieszczam tylko .clang-format, który warto byłoby umieścić w odpowiednim pliku, przy okazji.

---
BasedOnStyle : LLVM
AccessModifierOffset : '-4'
AlignAfterOpenBracket : DontAlign
AlignConsecutiveAssignments : 'false'
AlignConsecutiveDeclarations : 'false'
AlignEscapedNewlinesLeft : 'true'
AlignOperands : 'true'
AlignTrailingComments : 'false'
AllowAllParametersOfDeclarationOnNextLine : 'false'
AllowShortBlocksOnASingleLine : 'false'
AllowShortCaseLabelsOnASingleLine : 'false'
AllowShortFunctionsOnASingleLine : Empty
AllowShortIfStatementsOnASingleLine : 'false'
AllowShortLoopsOnASingleLine : 'false'
AlwaysBreakAfterReturnType : None
AlwaysBreakBeforeMultilineStrings : 'false'
AlwaysBreakTemplateDeclarations : 'true'
BinPackArguments : 'true'
BinPackParameters : 'true'
BreakBeforeBinaryOperators : None
BreakBeforeBraces : Linux
BreakBeforeTernaryOperators : 'false'
BreakConstructorInitializersBeforeComma : 'false'
BreakStringLiterals : 'false'
ColumnLimit : '200'
ConstructorInitializerAllOnOneLineOrOnePerLine : 'false'
ConstructorInitializerIndentWidth : '4'
ContinuationIndentWidth : '4'
Cpp11BracedListStyle : 'false'
DerivePointerAlignment : 'false'
DisableFormat : 'false'
IndentCaseLabels : 'true'
IndentWidth : '4'
IndentWrappedFunctionNames : 'false'
KeepEmptyLinesAtTheStartOfBlocks : 'false'
Language : Cpp
MaxEmptyLinesToKeep : '1'
NamespaceIndentation : All
PointerAlignment : Left
ReflowComments : 'false'
SortIncludes : 'false'
SpaceBeforeAssignmentOperators : 'true'
SpaceBeforeParens : ControlStatements
SpaceInEmptyParentheses : 'false'
SpacesBeforeTrailingComments : '1'
SpacesInAngles : 'false'
SpacesInCStyleCastParentheses : 'false'
SpacesInParentheses : 'false'
SpacesInSquareBrackets : 'false'
Standard : Cpp11
TabWidth : '4'
UseTab : ForIndentation
---