Wie sollen nicht funktionale Änderungen vorgenommen werden?

Ich arbeite eine kleine bis mittlere Legacy Code - Basis und wenn es auf einem Ticket arbeiten werde ich über Code kommen , die aufgeräumte werden soll oder dass wir brauchen , um clean-up nur in der Lage zu sein , die Folge der Anwendung zu verstehen.

Ein echtes Beispiel ist:

if( !a && b ){
     doSomething1();
     doSomething2();
     doSomething3();
     doSomething4();
     doSomething5();
}else if( a ){
     doSomething1();
     doSomething2();
     doSomething3();
     doSomething4();
     doSomething5();
}else if( !b && ( a || c )  ){
     doSomething1();
     doSomething2();
     doSomething3();
     doSomething4();
     doSomething5();
}

Eine andere Möglichkeit besteht darin, Tippfehler und Engrish in Kommentaren und Dokumentationen in einem Dutzend Quelldateien zu beheben.

Oft hängt diese Bereinigung jedoch nicht mit dem Hauptproblem zusammen, und ich frage mich, wie es am besten ist, die Bereinigung durchzuführen. So wie ich das sehe, gibt es drei Möglichkeiten:

1. Vor dem Fix: Dies funktioniert chronologisch, da dies die Reihenfolge ist, in der sie auftreten. Wenn sie jedoch etwas beschädigen, erschweren sie den Fix und es ist schwieriger, den Fix von dem zu unterscheiden, was in der Produktion war. Außerdem wird ein zusätzliches Commit eingeführt, das Rauschen ist.
2. Mit dem Fix: aber das verdeckt den tatsächlichen Ersatz von fehlerhaftem Code durch Dateien, wo findGeoragphydas richtig war findGeography.
3. Nach dem Fix: Dies erfordert das Entfernen und Bereinigen, das Sie vorgenommen haben, um den Code besser zu verstehen, und das erneute Testen des Fixes, das Festschreiben des Fixes und das Zurückgehen und Wiederholen der Bereinigung. Dies ermöglicht den saubersten Unterschied zum fehlerhaften Code, verdoppelt jedoch den Aufwand und kann auch zu falschen Commits führen.

tl; dr: Also, was ist die beste Methode, um Bereinigungscode festzuschreiben?

Kontext: Wir haben keine Unit-Tests und Entwicklungsfortschritte, indem wir Änderungen ausführen, sie in Augenschein nehmen und sie zur Qualitätssicherung über die Mauer werfen, um sie durch manuelle Regressionskorrekturen zu validieren. Wenn wir keine Formularbereinigungen durchführen, wird der Code unverständlich. Ich weiß, dass dies alles andere als ideal ist, aber dies ist ein echter Unternehmensentwickler, der Essen auf meinen Tisch legt und nicht meine Wahl.

Ich folge einem ähnlichen Prozess wie Karl. Ich bevorzuge aus mehreren Gründen ein separates Festschreiben der Bereinigung / Umgestaltung vor den Funktionsänderungen:

• Wenn Sie die beiden Commits trennen, wird Ihr Prozess erklärt und anderen, die sich die Protokolle ansehen, besser gedacht.
• Es enthält eine Reihe spezifischer Änderungen, die während einer Codeüberprüfung hilfreich sind.
• Wenn Sie Ihre Funktionsänderungen rückgängig machen möchten, behalten Sie die Bereinigung / den Refactor bei.
• Die Bereinigungsänderungen können separat und vor Funktionsaktualisierungen getestet werden.

Ich bevorzuge das zusätzliche Commit vor dem Fix. Dadurch werden die beiden Aufgaben, die Sie ausführen, klar voneinander getrennt, und andere Personen können sehen, was bereinigt wurde und was eine Fehlerbehebung war. Außerdem ist die Bereinigung (zumindest für mich) normalerweise viel schneller. Wenn ich also eine Bereinigungskorrektur durchführe, muss ich mir nicht so viele Sorgen machen, dass jemand eine schwer zusammenführbare Änderung vornimmt, während ich länger arbeite Aufgabe.

Ich habe den Ansatz "Commit with the Fix" an Stellen mit Richtlinien verwendet, für die bei jedem Commit eine Änderungsanforderungs-ID erforderlich ist, und Entwickler dürfen keine neuen Änderungsanforderungen nur für Bereinigungscode erstellen. Ja, solche Richtlinien sind unproduktiv, aber es gibt solche Arbeitsplätze.

Wenn das Update möglicherweise in eine vorhandene Version gepatcht werden muss, sollten Sie zuerst das Update und anschließend die Bereinigung einchecken, oder Sie machen (zumindest zeitweise) die Person, die das Update anwenden muss Arbeiten Sie härter als nötig (entweder um zu verstehen, was sich geändert hat, oder um die kosmetischen Änderungen zu patchen, bevor Sie den eigentlichen Fix patchen) (Ich werde feststellen, dass ich mehr als ein paar angeblich kosmetische Änderungen vorgenommen habe, die letztendlich etwas kaputt gemacht haben, also mehr als patchen Die minimale Änderung zur Behebung eines Defekts kann das Risiko erhöhen, etwas Schlechtes freizugeben.

Ja, das ist zusätzliche Arbeit. Ja, es ist ein Schmerz. Aber ja, es ist der richtige Weg, Dinge zu tun. Mein normaler Arbeitsablauf besteht darin, alles in einer Sandbox zu erledigen (Bereinigen und Beheben von Fehlern), dann eine neue Sandbox zu erstellen, nur die minimale Änderung der Fehlerbehebung anzuwenden, diese Änderung in der ursprünglichen Sandbox zu überprüfen und die Bereinigung einzuchecken. up Code. Nach meiner Erfahrung ist es nicht so viel zusätzliche Arbeit.