Hallo zusammen, haltet ihr Code Reviews, welche vom Autor selbst getätigt werden für sinnvoll? MfG Der Frager
Dreister Frager schrieb: > Code Reviews, welche vom Autor selbst getätigt werden für sinnvoll? So sinnvoll wie sich selbst die Haare zu schneiden... mit der Schere...
Wenn zwischen Schreiben und Review einige Jahre bis Jahrzehnte liegen kann es lustig sein. Spaß ist sinnvoll.
Dreister Frager schrieb: > haltet ihr Code Reviews, welche vom Autor selbst getätigt werden für > sinnvoll? Das ist bis zu dem Punkt sinnvoll, bis zu dem er alles, was er an Anforderungen zum Test bekommen hat, auch abarbeitet und nachweist, das es funktioniert. Das gilt auch für die Selbstgestellten Vorgaben. Offen bleiben dabei zwei Dinge: 1) Was hat er alles ab Vorgaben zum Testen und Reviewen nicht gemacht, weil er es übersehen hat? 2) Was hat er alles nicht richtig gemacht und übersieht es beim optischen Inspizieren? Beides ist kritisch. Das self review ist damit nur eine Teilabdeckung des reviews, sollte im review sheet auch entsprechend ersichtlich sein, z.B. durch punktweises Abhaken. Dann sieht jeder, was noch offen ist. wir machen 4-Augen-Reviews, d.h. alles was einer beguckt hat, beschaut sich nochmal ein zweiter. Und zwar NICHT gleich zeitig!
Beitrag #6262204 wurde von einem Moderator gelöscht.
Ja, natürlich. Das ist ähnlich wie alle Checklisten: gut, wenn es gemacht wird, besser wenn es ein anderer macht. Die allermeisten Berichte oder Formalien sind doch genau dafür da, einen geordneten Abschluss zu bekommen. Sonst heißt es immer: dazu bin ich noch nicht gekommen.
Dreister Frager schrieb: > haltet ihr Code Reviews, welche vom Autor selbst getätigt werden für > sinnvoll? Nein, das sind keine 'Code reviews' sondern simpler 'Cargo Cult' - man ahmt etwas nach, dessen Sinn und Wirkungsweise man nicht verstanden hat. Das fängt schon damit an, das man die 'roles' beim Review (Programmierer,Gutachter, Protokollant) nicht benannt hat und geht weiter über die Nicht-Bennung der Ein- und Ausgangsdokumente (Spec, Style-Code, Code, Änderungsbeschreibung, To-Do-Liste, Review-Protokoll). Wahrscheinlich ist nur dem kleinsten Teil der Programmierer bewußt, das es für Reviews auch Standards wie IEEE-1028 und IEEE-730 gibt und man sich das genaue Procedere nicht aus den Finger saugen muss. Oft kann aber der gemeine Coder nichts dafür, das die Reviews zur reinen Augenwischerei werden, es liegt bei dem Manager dafür zu sorgen, das der Review auch die Qualität des Produktes verbessert und das die Coder entsprechend geschult sind. Wobei 'Schulung' schon mit der Ausfertigung einer Checklist/Ablaufplan für den Review getan sein könnte - und diese Checklist/Pläne muß der Manager liefern. Oder die Schulung.
Kimonopython schrieb: > Das fängt schon damit an, das man die 'roles' beim Review > (Programmierer,Gutachter, Protokollant) nicht benannt hat und geht > weiter über die Nicht-Bennung der Ein- und Ausgangsdokumente (Spec, > Style-Code, Code, Änderungsbeschreibung, To-Do-Liste, Review-Protokoll). In typischen deutschen Großunternehmen (und auch kleineren Unternehmen mit vergleichbaren Strukturen) wird die Erstellung und Verwaltung solcher Dokumente dann aber zum Selbstzweck erhoben, so dass es dann gar nicht mehr um die entsprechenden Inhalte geht, sondern nur noch die Einhaltung der Formalien bei der Dokumentenerstellung geht. Die damit einhergehenden Prozesse werden dann natürlich auch nicht mehr bezüglich ihrer Eignung für den eigentlichen Zweck bewertet, nämlich die Produktverbesserung, denn daran hängen doch etliche Pöstchen. Und da ja auch sehr viel Arbeit hineingeflossen ist, müsste sich ggf. auch derjenige, der den Prozess eingeführt hat, dafür verantworten, wenn die ganze Arbeit für die Tonne gewesen wäre. Vor nicht allzu langer Zeit hatte ich von einem Kunden solch ein Konvolut an reinen Blabla-Texten bekommen. Dieses sollte als Vorlage für die konkreten Produktentwicklungen dienen. Auf meine sehr deutlichen Einwände, dass diese Dokumente völlig unbrauchbar wäre, hieß es nur, ein bestimmter Mitarbeiter hätte mehrere Jahre an diesen Vorlagen gearbeitet. Niemand kam damit zurecht. Ca. zwei Jahre später wurde der Autor dann in den Vorruhestand geschickt. Als ich bei dem letzten Projekt fragte, ob wieder diese Dokumente erstellt werden müsste, hieß es nur: "Nein, jetzt ist X. ja in Rente." > Oft kann aber der gemeine Coder nichts dafür, das die Reviews zur reinen > Augenwischerei werden, es liegt bei dem Manager dafür zu sorgen, das der > Review auch die Qualität des Produktes verbessert und das die Coder > entsprechend geschult sind. Wenn es den überhaupt zu einem wie auch immer gearteten Review kommt, ist doch schon viel gewonnen. Wenn stattdessen nur Spezifikationen darüber verfasst werden, wie solch ein Review aussehen müsste, wenn man es denn durchführen würde, und das Review dann nur für diese Dokumente durchgeführt wird, aber niemals das Code Review stattfindet, hat man einfach nur Zeit verschwendet, die man lieber für die Entwicklung und nicht "nicht normgerechte" eigene Kontrollen verwendet hätte. Bei einem anderen Kunden habe ich nämlich genau diese Situation erlebt. Ein Entwickler war monatelang damit beschäftigt, solche Dokumente insbesondere auch für meine Software zu erstellen, ohne auch nur einen Blick auf den Quelltext zu werfen. Irgendwann war er damit fertig, und ich fragte ihn ganz verwundert, wann denn das eigentliche Review stattfände. Er meinte daraufhin, dass es gar nicht mehr erforderlich sei, denn für die Zulassung des Produktes müsse nur die Vorgehensweise dokumentiert sein, aber nicht die eigentliche Durchführung erfolgen.
Andreas S. schrieb: > Er meinte daraufhin, dass es gar nicht mehr erforderlich sei, denn für > die Zulassung des Produktes müsse nur die Vorgehensweise dokumentiert > sein, aber nicht die eigentliche Durchführung Ha-ha! Schildbürgertum live! Entweder wird so etwas nicht verstanden und einfach für den Selbstzweck (oder Befriedigung) erstellt, wie du sagst, oder man bedient damit irgendwelche Bürokraten und Verwalter in solchen Konzernen oder anderen Unternehmen mit starren Strukturen...! Deutschland begräbt sich unter ISO Normen, die halt einfach als purer Formalismus erfüllt werden. Effektiv und inhaltlich bringts nix, gar nix. Hab das selbst schon oft erleben müssen. Na dann, gute Nacht, Deutschland!
Andreas S. schrieb: >> Oft kann aber der gemeine Coder nichts dafür, das die Reviews zur reinen >> Augenwischerei werden, es liegt bei dem Manager dafür zu sorgen, das der >> Review auch die Qualität des Produktes verbessert und das die Coder >> entsprechend geschult sind. > > Wenn es den überhaupt zu einem wie auch immer gearteten Review kommt, > ist doch schon viel gewonnen. viel falsche Sicherheit ist gewonnen ... Also statt 'wie immer geartet' dann 'mit absolut notwendigen Aufwand' Und der wäre: -Vorbereitung (Code und passende Doc/Spec aus-checken; comments im code ggf. überarbeiten, anlegen; log-files vom built bereithalten; ...) -Durchführung (gemeinsames 'durchwandern' des Codes - 'was macht der Code hier' und 'warum') -Email-protokoll mit Angabe wer hat welchen Code (tag/versionsnummer aus der versionsverwaltung, filenamen) nach welchen Kriterien (spec oder wenigstens kurze Funktionsbeschreibung) aus welchen beweggrund (Fehlerreport) durchgesehen. Wenn es keinen konkreten Beweggrund gibt, wird auch kein Review durchgeführt. Ebenso wenn es keine Kriterien (bspw. style-guide,(Mini-)Spec) gibt. dann muss dieser Style-Guide eben erstellt werden. Oder man nimmt einen und passt den an. Man kann auch das Compiler-log verwenden in dem Sinne, alle warnings (natürlich bei -Wall) sind gelesen und verstanden. Man kann auch dem Programmierer das Buch ISBN: 978-3-89721-567-2 geben und Seiten 61-86 zur Pflichtlektüre (während der bezahlten Arbeitszeit) erklären.
Kimonopython schrieb: > -Durchführung (gemeinsames 'durchwandern' des Codes - 'was macht der > Code hier' und 'warum') Das scheitert in vielen Fällen doch schon am "gemeinsam". Ein Entwickler bleibt auf sich gestellt. Derjenige, der als Kontrolleur infrage käme, ist durch bürokratische Formalismen gebunden. So ist die Realität in vielen Unternehmen. > Wenn es keinen konkreten Beweggrund gibt, wird auch kein Review > durchgeführt. Wie schon geschrieben, kommt es vor, dass die Vorgehensweise dokumentiert wird, ohne anschließend dementsprechend vorzugehen. > dann muss dieser Style-Guide eben > erstellt werden. Oder man nimmt einen und passt den an. Und dabei kommen dann eben ggf. diese völlig unbrauchbaren Dokumente und Vorlagen wie zuvor erwähnt heraus. Solche Dokumente werden ja häufig von Leuten verfasst, die entweder niemals selbst entwickelt oder sich als fachlich entbehrlich erwiesen haben. Hinzu kommt, dass solche Leute auch keine Projektverantwortung tragen, da sie ihre Projekte schon Jahre vorher verloren haben. Die erstellten Pamphlete passen dann weder zu den aktuellen Projekten, Entwicklungsprozessen, Werkzeugen, Programmiersprachen, "best practises". Wer sich daran hält, hat verloren. Natürlich ist das nicht bei jedem Unternehmen so, aber bei der deutlichen Mehrzahl meiner Kunden erlebe ich genau dies in unterschiedlich starker Ausprägung. Und je größer und etablierter das Unternehmen ist, desto schlimmer ist es.
Einen guten Code-Review kann nur ein Code-Review-Spezialist durchführen. Nur er weiss, auf was er achten muss und wo die üblichen Nachlässigkeiten der Coder lauern. Er betrachtet das ganze Szenario aus einer anderen Perspektive. Natürlich muss der Code-Reviewer auch unabhängig sein, um ein bestmögliches Ergebnis sicherzustellen, also nicht mit dem Entwickler sozialisiert (Talk in der Kaffeeküche). Wir setzen deshalb für unsere Code-Reviews als auch für unsere Code-Audits nur externe Spezialisten ein. Diese sind schon für ein Stundenhonorar für 150 Euro buchbar. Natürlich fallen für die Code-Reviews deutlich weniger Stunden an als die Entwickler benötigen, wir kalkulieren meist mit 1:8 so dass die Kosten normalerweise nicht zu sehr ins Gewicht fallen.
Andreas S. schrieb: > Natürlich ist das nicht bei jedem Unternehmen so, aber bei der > deutlichen Mehrzahl meiner Kunden erlebe ich genau dies in > unterschiedlich starker Ausprägung. Und je größer und etablierter das > Unternehmen ist, desto schlimmer ist es. .. und mm schlimmsten ist es beim Grössten, bei Boeing: https://www.theguardian.com/us-news/2019/dec/11/boeing-737-max-plane-faa-regulators-crash-risk >Das scheitert in vielen Fällen doch schon am "gemeinsam". Ein Entwickler >bleibt auf sich gestellt. Derjenige, der als Kontrolleur infrage käme, >ist durch bürokratische Formalismen gebunden. Also wird der angelieferte Code einfach durchgewunken und nicht abgenommen ... aber solange gezahlt wird?! Ja, ich weiß das ist nicht einfach. Da muß wohl jeder selbst einen Weg finden um mit sich und den Qualitätsansprüchen des Kunden seinen Frieden zu machen.
Also ich lese hier ein paar Dinge, die fuer mich sehr befremdlich klingen (aus der Perspektive in einer grossen US IT Firma): Ingenieur24 schrieb: > Einen guten Code-Review kann nur ein Code-Review-Spezialist durchführen. Kimonopython schrieb: > Email-protokoll ... > Wenn es keinen konkreten Beweggrund gibt, wird auch kein Review > durchgeführt. Vielleicht sollte man erst mal die Begriffe klaeren, um sicherzustellen dass alle ueber das selbe reden? - Unter "Code Review" verstehe ich ein "peer review", bei dem Kollegen die mit der Materie vertraut sind eine Aenderung ueberpruefen und absegnen bevor sie im zentralen Repository landet. Dazu werden Tools wie gerrit verwendet. Das ist nicht optional, die genauen Ablaeufe (noetige Anzahl der Approvals, wer kann approven) kann aber jedes Team fuer sich festlegen. - Unter "Audit" verstehe ich eine anlassbezogene Pruefung auf bestimmte Aspekte (z.B. zur Sicherheitszertifizierung). Da wuerde dann ein Team-externer Pruefer beauftragt.
Andreas S. schrieb: > Solche Dokumente werden ja häufig von > Leuten verfasst, die entweder niemals selbst entwickelt oder sich als > fachlich entbehrlich erwiesen haben. Das ist meist das eigentliche Problem. Und selbst wenn man Spezialisten heuert, wie hier Ingenieur24 schrieb: > Wir setzen deshalb für unsere Code-Reviews als auch für unsere Code-Audits > nur externe Spezialisten ein. macht das nur Sinn, wenn der Background der Leute zur Aufgabe und Umfang (nicht nur Sprache oder Plattform) passt und ein "unerfahrenes" Team diszipliniert und erzogen wird. Am effektivsten ist ein Codereview natürlich intern, wenn ein gemeinsames Verständnis der Best-Practices entwickelt wird, und alle daran wachsen. Es muss nur sichergestellt sein, dass auch wirklich ein paar intelligente oder gute Leute dabei sind.
Beitrag #6263097 wurde von einem Moderator gelöscht.
Selbstkontrolle / entsprechende Tests gehören eh immer dazu. Sofern es sich nicht um ein 1-Mann-Projekt handelt, sollte ein Kollege das Code-Review übernehmen. Alles andere wäre irgendwie albern. Ist wie als wenn du ohne Musterlösung deine eigene Klausur kontrollierst.
Jennifer R. schrieb: > Alles andere wäre irgendwie albern. Ist wie als > wenn du ohne Musterlösung deine eigene Klausur kontrollierst. Willkommen in der Realität! Um den Vergleich mit einer Klausur zu bemühen: Wenn man von der Thematik keine Ahnung hat, reicht es aus, einen länglichen Text zu schreiben, in dem zunächst einmal möglichst abstrakt und unverbindlich festgelegt wird, was eine Klausur überhaupt ist. Hierfür werden möglichst ungebräuchliche und außerhalb des Kontextes dieses Textes unverständliche Bezeichnungen in der Langform und entsprechende Abkürzungen eingeführt. Dann beschreibt man unter exzessiver Anwendung dieser Begriffe, in welchen nachrangigen Dokumenten die Struktur der Klausur-Aufgabestellung und die Struktur der sich daraus ergebenden Anforderungsdokumente definiert werden. Dann wird beschrieben, in welchen nachrangigen Dokumenten das Vorgehen für die Erstellung der korrespondierenden Prüfspezifikationen auf Klausurebene beschrieben werden. Wichtig ist bis jetzt, jeglichen Bezug auf die eigentliche Klausur zu vermeiden. Das ganze wird dann rekursiv auf der Komponentenebene, die den Aufgabenblöcken entsprechen, wiederholt. Und dann noch einmal das ganze auf der Modulebene, die dann den Fragen bzw. Aufgaben entsprechen. Da das ganze etwas unübersichtlich geworden sein könnte, folgt nun ein Übersichtsdokument über die bisher erstellten Dokumente. Da es praktisch wäre, das Übersichtsdokument gleich am Anfang zu platzieren, wird eine Beschreibung zur Erstellung von Änderungsanweisungen erstellt. Dies wird auch gleich umgesetzt, nämlich in Form einer Änderungsanweisung für die Dokumentenreihenfolge. Leider ändern sich dadurch die vergebenen Dokumentennummern; das in diesem Fall anzuwendende Vorgehen folgt in der Dokumentennummernänderungsverfahrensbeschreibung festgelegt. Diese wird auch sofort angewendet; leider enthält sie einen kleinen Fehler und führt dazu, dass die Dokumentennummern völlig inkonsistent vergeben werden. Spätestens an diesem Punkt hat der Korrektor keine weitere Zeit mehr, die Klausur zu korrigieren. Da bis dahin alles schlüssig erscheint, gibt es eine (in Schulnoten) fette 1. Auf Grund der fehlerhaften Dokumentennummern wird sie auf eine 1- abgewertet; wenn der Korrektor schlecht geschissen hat, auf eine 2+.
:
Bearbeitet durch User
Code review ... oftmals waere es schon gut man koennte Konzepte besprechen um zu wissen ob man ueberhaupt auf dem richtigen Gleis ist. Der Code muss nicht wirklich angeschaut werden, solange der Zug in die richtige Richtung fahrt passt das schon.
Ab und an sind Code Reviews eine ganz gute Sache. Allerdings sollten dies 2 verschiedene Personen gegenseitig machen und darüber reden. Man kann dabei immer wieder was lernen. Allerdings ist dies allgmein viel zu Zeitaufwändig und auch nicht nötig, aber ab und an wenn etwas mehr Zeit vorhanden ist eine super sache...
Mal einwerfen schrieb: > oftmals waere es schon gut man koennte Konzepte besprechen um zu wissen > ob man ueberhaupt auf dem richtigen Gleis ist. Der Code muss nicht > wirklich angeschaut werden, solange der Zug in die richtige Richtung > fahrt passt das schon. Klingt nach einem Design Workshop. Was spricht dagegen, sowas zu organisieren?
Mal einwerfen schrieb: > Wenn die Leute denn verfuegbar waeren ... die machen alle was > anderes Dann schnapp sie dir!
Bei vielen Projekten wär's ja schon ein Gewinn, wenn auch nur Compilerwarnungen aktiviert würden (z.B. GCC: -Wall -Wextra) und das Projekt verpflichtend ohne Warnungen durchcompilieren muß.
Thread beobachten
Seitenaufteilung abschaltenBitte melde dich an um einen Beitrag zu schreiben. Anmeldung ist kostenlos und dauert nur eine Minute.
Bestehender Account
Schon ein Account bei Google/GoogleMail? Keine Anmeldung erforderlich!
Mit Google-Account einloggen
Mit Google-Account einloggen
Noch kein Account? Hier anmelden.