Czy warto zatwierdzać wyłącznie w celu rozwiązywania niekrytycznych literówek?

67

Jeśli napotkam niekrytyczną literówkę w kodzie (powiedzmy, błędną apostrofę w instrukcji print (error)), czy warto podjąć zobowiązanie, aby rozwiązać ten błąd, czy może po prostu należy go zostawić w spokoju?

W szczególności jestem ciekawy, jak porównać przeglądanie dziennika zmian do wartości rozwiązania tych niekrytycznych literówek. Skłaniam się do ich rozwiązania. Czy jestem pedantyczny?

Christopher Berman
źródło
40
Jeśli trywialne zatwierdzenie cokolwiek zepsuje, musisz albo zainwestować w lepszy VCS, lepsze narzędzia do filtrowania dzienników lub lepsze praktyki w zakresie notowania różnych poziomów poprawek.
Rex Kerr
@ root45 Chociaż jeśli przyjrzysz się wynikom, są one innego rodzaju gramatyką niż ta, o którą pyta to pytanie.
Izkata,

Odpowiedzi:

130

Osobiście uważam, że poprawa jakości jest warta drobnych niedogodności związanych z dodatkowym wpisem w dzienniku zatwierdzeń, nawet w przypadku drobnych ulepszeń. W końcu małe ulepszenia liczą się bardzo, gdy weźmie się pod uwagę efekt rozbicia okna .

Możesz chcieć poprzedzić go TRIVIAL:tagiem lub oznaczyć jako trywialny, jeśli twój VCS go obsługuje.

Thiton
źródło
72
Chcę tylko dodać do tego, że wolałbym, aby takie rzeczy były popełniane osobno, niż łączone z czymś innym. Problem polegający na zbijaniu polega na tym, że zbyt duży hałas może odwrócić uwagę recenzenta kodu od odpowiednich zmian. +1
Andy,
9
+1 za wzmiankę o efekcie rozbicia okna. Małe rzeczy mają znaczenie. Jeśli wszystko jest naprawdę czyste, ludzie pomyślą dwa razy, zanim popełnią niedbale napisany lub nieprzetestowany kod.
Roy Tinker,
25
Również jeśli dodasz go do elementu za pomocą funkcji, a następnie przywróć funkcję, utracisz zmianę. Popełniaj jedną rzecz na raz.
ctrl-alt-delor
4
Byłbym bardzo zainteresowany, które VCS pozwalają na oznaczanie komentarzy jako trywialnych. Pracowałem z SVN, Hg i Git i niczego takiego nie zauważyłem.
Xion
3
@Andy, że hałas nie jest najgorszą częścią ... jeśli ktoś później zdecyduje się na przykład cofnąć to zatwierdzenie, ponieważ nie chce tej funkcji, nagle tracisz również niewielką poprawę, która była częścią zatwierdzenia (zatwierdzeń).
rFactor
49

Nie jesteś pedantyczny i lepiej rozwiązywać je indywidualnie. Im bardziej atomowa jest zmiana, tym lepiej - nie chcesz, aby naprawiana usterka była mieszana z 500 zmianami komentarzy / literówek.

jmoreno
źródło
9
+1 Każda wersja powinna odnosić się tylko do jednej konkretnej poprawki. Poprawki do backportu stają się NIGHTMARE, jeśli każda wersja ma również kilka losowych literówek między prawdziwymi zmianami.
Grant
28

W ogólnym przypadku: tak

To zawsze warto zwiększyć konserwacji swojego oprogramowania.

Po prostu to zrób.

Jeśli masz zamiar wysłać wydanie ...

... a jeśli nie jesteś liderem zespołu, skontaktuj się z nim / nią .


Odnośnie zawartości dziennika zmian ...

Zgadzam się z innymi, że powinieneś przynajmniej napisać coś, co odróżnia go od zatwierdzeń związanych z „cechą”, jeśli chodzi tylko o naprawienie pojedynczej literówki.

Powszechną praktyką jest wykonywanie w narzędziu do śledzenia problemów niektórych niekończących się zadań w celu śledzenia ponadczasowych i niekończących się zmian. Na przykład często zdarza się, że masz zadanie:

  • duże zautomatyzowane nieszkodliwe porządki (zamiatanie białych znaków),
  • polowania na gramatykę i literówki,
  • budować modyfikacje systemu,
  • itp...

Tylko bądź ostrożny, aby nie były one używane jako wyrzucane identyfikatory zadań dla prawie wszystkiego, gdy ludzie leniwie robią poprawnie udokumentowane bilety. Zwłaszcza jeśli odrzucisz zatwierdzenia, które nie są powiązane z identyfikatorem (to dobrze, ale duże zadania takie będą jeszcze bardziej atrakcyjne dla leniwych programistów).

Haylem
źródło
7
Sprawdzasz u swojego lidera zespołu poprawkę literówki? Gdybym był liderem zespołu, który jest zestresowany nadchodzącym wydaniem, nie chciałbym, aby przeszkadzały mi takie błahostki.
Joh
2
@Joh: nie chodzi o to, że możesz zepsuć kompilację, ale mogą istnieć inne zadania kompilacji lub jedno może być już otagowane i gotowe do użycia, a kontrola kontroli (w zależności od branży) może zmusić cię do powiązania tego nieszkodliwego zatwierdzenia do zadań i wymagań, więc jeśli pojawi się znikąd, może to być tylko ból głowy; które można łatwo zapisać na kilka dni po wydaniu. Ale ogólnie zgadzam się z tobą, a nawet powiedziałbym, że firma działająca w ten sposób robi to źle. Ale nie jesteś tym, który to naprawi, więc prowadź to, jeśli nie jesteś starszy.
haylem
2
@Joh: Zasadniczo, jeśli mój lider zespołu zauważy, że nowa kompilacja nagle podskakuje w systemie kompilacji, gdy ma zamiar rozpocząć nową kompilację (lub już ją rozpoczęła), to mogę zapewnić, że jego poziomy stresu wzrosną, stosując takie podejście również ...
szlem
7

Literówki należy dodać jako zatwierdzenie. Naprawa błędnie napisanych słów lub błędów gramatycznych zwiększy czytelność twojego kodu.

Użycie komunikatu zatwierdzenia, takiego jak „Naprawiono literówkę” lub „Naprawiono literówkę w pliku.c” pomoże ci odróżnić te zatwierdzenia od innych, głównych zatwierdzeń kodu.

Trevor
źródło
7

Tak, absolutnie powinieneś to zrobić, szczególnie na początku projektu.

Dlaczego? Dwa punkty:

  1. Prawdopodobnie nie będziesz wiedział, czy literówka jest „krytyczna”, czy nie, dopóki nie będzie za późno. Niektóre poprawki prawdopodobnie nie zostaną naprawione, ponieważ wszyscy myślą, że to nie będzie wielka sprawa. Aż tak będzie.

  2. Wcześniejsze i celowe naprawianie literówki będzie znacznie łatwiejsze niż naprawianie po kilkuset wierszach wywołań kodu / funkcji. Ponownie, tymczasowe włamania mogą zaskakująco szybko stać się półtrwałe. Dlatego mam do czynienia z obiektami, które mają zarówno metody „CollapseAll” ORAZ „ColapseAll”.

joshin4colours
źródło
2
Tak - spośród wszystkich poprawnych odpowiedzi podoba mi się ta najbardziej, ponieważ wspomina, że ​​zależy to od „kiedy”.
Wonko the Sane
4

W przypadku błędów gramatycznych, które mogą być widoczne dla użytkownika końcowego, to tak, za wszelką cenę warto dokonać zatwierdzenia, ponieważ jest całkiem możliwe, że użytkownik lub QA może przyjść i zgłosić błąd i będzie musiał być śledzony. Jeśli jest już naprawiony, może przyspieszyć czas potrzebny do rozwiązania problemu.

Jeśli jednak jest to błąd gramatyczny w komentarzach do kodu, nie zrobiłbym nic, chyba że jest to część zmian w rzeczywistym kodzie, w którym to przypadku aktualizujesz dokumentację kodu.

rjzii
źródło
3

Jeśli martwisz się o zebranie dziennika zmian, robisz coś innego źle. :-) Częste zmiany są dobrą rzeczą! Cały czas popełniam poprawki literówek. Wkrocz do bazy kodów jak najszybciej i przyśpiesz cykl tworzenia aplikacji!

Brian Knoblauch
źródło
2
I commit typo fixes all the timeIMO to niepokojące, spróbuj znaleźć programistów z lepszym angielskim?
Lie Ryan,
Weź to, co możesz dostać w tych dniach. Znalezienie programistów, którzy potrafią w ogóle napisać sprawny kod, to poważny problem!
Brian Knoblauch,
2

Głosuję tak. Zamelduj się. Pracowałem dla firmy, która nienawidziła osób sprawdzających rzeczy. Mam na myśli prawie wszystko. Recenzje kodu były obszerne, więc jeśli zarejestrowałeś zmianę, która naprawiła literówkę, na którą narzekałeś. Możesz sobie wyobrazić stan kodu. W rzeczywistości kod nie był straszny, ale płynął jak melasa, a nie płynął jak wino.

Ian
źródło
0

Czy proces zarządzania zmianami na to pozwala?

W moim środowisku każde zatwierdzenie, które dokonuję, musi być powiązane z żądaniem zmiany, o które poprosili użytkownicy biznesowi lub obowiązkowa zmiana na poziomie systemu, wraz z odpowiednimi procesami testowania użytkownika końcowego. Prosta literówka, którą opisujesz, prawdopodobnie nie zostałaby zapisana jako jedna z tych (znalazłem literówki i błędy gramatyczne w jednej z moich aplikacji, które istniały przez ponad 4 lata, nikt tego nie zauważył), więc jeśli / kiedy przyjdą audytorzy dzwoniąc, miałbym bardzo trudne wytłumaczenie siebie.

Zapisałbym zmianę, którą opisujesz (a tak naprawdę ją mam - nazwa metody jest błędnie napisana i właśnie ją odkryłem) na czas, kiedy mam „prawdziwą” zmianę, która musi być wykonane również i umieścić w dzienniku „naprawiono różne literówki”.

alroc
źródło
+1 W niektórych środowiskach jest to prawda. Proszę nie głosować tylko dlatego, że nie jest to prawdą w miejscu pracy.
MarkJ
-1 wydaje się, że proces zarządzania zmianami jest bardzo pracochłonny . Rozumiem (a nawet wolę ), że każde zatwierdzenie musi być powiązane z żądaniem zmiany - ta część twojego procesu wygląda dobrze. Ale OMG wygląda na to, że żądania inicjowane przez programistę (takie jak zmiana tego / poprawiania literówek w tym ) są zabronione, co powoduje, że podchodzę do dużej czerwonej flagi . Zakłada się, że użytkownicy / audytorzy biznesowi zawsze wiedzą lepiej niż programiści - jeśli byłoby to kiedykolwiek bliskie prawdy, wtedy sami napisaliby kod
skomentowali
Jeśli programista może przekonać osoby, które wyrażają zgodę na wprowadzenie zmian, które warto wprowadzić, mogą przejść dalej. Ale jeśli znajdę prostą literówkę w nazwie metody lub znajdę jakiś kod, który należy skopiować / wkleić i który powinien zostać przekształcony w metodę, nie mogę dokonać zmiany bez autoryzacji. Nie chodzi tylko o „właściwe postępowanie z kodem” - należy przeprowadzić testy akceptacyjne, a programiści nie są w stanie powiedzieć ludziom biznesu „Dokonałem tej zmiany, nigdy nie zauważysz, co ja” tak, ale musisz przejść cykl testowy ”.
alroc
@alroc, że „jeśli-można-przekonać” po prostu ukrywa proces przeciwny do zamierzonego pod rozsądnie wyglądającą powierzchnią. Wymóg przekonania biznesu / audytu o dużych zmianach, uzgodnionych punktach kontrolnych / wydaniach itp. Ma sens, OK. Poświęcenie kilku godzin / dni na usprawiedliwienie wysiłku, który zajmuje tydzień poświęcony na opracowanie i kontrolę jakości, jest żmudne, ale sprawiedliwe, OK. Ale zmusza to do każdego poprawek pisowni lub metody ekstraktu ... daj spokój - to nic innego jak kontrola bezmyślnie włożonej w niewłaściwym etapie w niewłaściwym czasie
komara
Pozwól mi spróbować wyjaśnić to jeszcze raz. Jeśli mogę poprawić literówkę lub wyodrębnić metodę podczas pracy nad inną, zatwierdzoną zmianą, mogę ją wsunąć bez konieczności sprzedaży. Ale nie mogę dokonać zmiany, która nie jest widoczna dla użytkownika i nie ma bezpośredniego pozytywnego wpływu na firmę / użytkowników, bez uprzedniej sprzedaży uprawnień, które będą na niej działać.
alroc
-1

Użyj DVCS do edycji historii

Jeśli obawiasz się o czystą historię zatwierdzeń, rozważ wykonanie głównej pracy w gałęziach funkcji . Jeśli zdarzy ci się pracować z rozproszonym VCS , możesz łatwo edytować swoją historię zatwierdzeń przed przekazaniem jej do głównej gałęzi. Jeśli korzystasz z SVN, wypróbuj Git - może on współdziałać dwukierunkowo z Subversion, a także możesz edytować historię, zanim przystąpisz do Subversion.

W przeciwnym razie zachowaj zdrowy rozsądek

Jeśli nie chcesz lub nie możesz edytować historii zatwierdzeń, nie ma funkcjonalnego powodu, aby zrobić wczesne lub atomowe zatwierdzenie niewielkiej literówki , która nie wpływa na automatyczne testy lub kompilację . W tym przypadku, moim zdaniem, utrzymywanie historii zmian w czystości powinno być ważniejsze niż dokonywanie naprawdę atomowych zmian. Mieszanie jednej lub dwóch poprawek literowych z „zwykłą” modyfikacją nie zaszkodzi żadnemu potencjalnemu procesowi przeglądu. Możesz jednak zgrupować kilka trywialnych poprawek w jednym zatwierdzeniu, być może podczas „czyszczenia” po większej sesji kodowania.

Zauważ, że błędy funkcjonalne nadal powinny być zatwierdzane jak najszybciej w zatwierdzeniu atomowym.

Ogólny ton odpowiedzi tutaj sugeruje strategię „szybko wszystko zatwierdzaj” nawet w przypadku niewielkich literówek. Zwykle nie zgadzam się i witam dyskusję.

krlmlr
źródło