Czy złą praktyką jest nie usuwać zbędnych plików od razu z VCS, ale zamiast tego oznaczać je jako „Do usunięcia” z komentarzami?

Chciałem wiedzieć, czy sposób postępowania z plikami źródłowymi, które należy usunąć z kontroli wersji, można uznać za złą praktykę.

Chcę ci to wyjaśnić na podstawie tego przykładu:

Niedawno bardzo się zdenerwowałem, ponieważ musiałem żmudnie sortować klasy Java w programie, który był w zasadzie martwym kodem, jednak nigdzie go nie udokumentowano, a także nie skomentowano w tych klasach Java. Oczywiście trzeba je było usunąć, ale zanim usunę takie zbędne rzeczy, mam - niektórzy mogą powiedzieć dziwny - nawyk:

Nie usuwam takich zbędnych plików natychmiast za pomocą SVN-> Usuń (zamień na polecenie usuwania wybranego systemu kontroli wersji), ale umieszczam komentarze w tych plikach (odsyłam zarówno do nagłówka, jak i stopki), które zamierzają zostać usunięte + moje imię + data, a także - co ważniejsze - DLACZEGO SĄ USUNIĘTE (w moim przypadku, ponieważ były martwe, mylący kod). Następnie zapisuję i przekazuję je do kontroli wersji. Następnym razem, gdy muszę zatwierdzić / sprawdzić coś w projekcie do kontroli wersji, wciskam SVN-> Usuń, a następnie są one ostatecznie usuwane w Kontroli wersji - wciąż oczywiście można je przywrócić poprzez poprawki i dlatego przyjąłem ten nawyk.

Dlaczego warto to robić zamiast usuwać je od razu?

Moim powodem jest to, że chcę mieć wyraźne znaczniki przynajmniej w ostatniej wersji, w której istniały te zbędne pliki, dlaczego zasługiwały na usunięcie. Jeśli je od razu usunę, zostaną usunięte, ale nigdzie nie udokumentowano, dlaczego zostały usunięte. Chcę uniknąć typowego scenariusza takiego jak ten:

„Hmm… dlaczego te pliki zostały usunięte? Wcześniej działałem dobrze.” (Presses „revert” -> facet, który cofnął, odszedł na zawsze lub nie będzie dostępny w ciągu najbliższych tygodni, a następny cesjonariusz musi dowiedzieć się żmudnie jak ja, o co chodzi w tych plikach)

Ale czy nie zauważasz, dlaczego te pliki zostały usunięte z komunikatów zatwierdzania?

Oczywiście, że tak, ale komunikat zatwierdzenia czasami nie jest czytany przez kolegów. Nie jest typową sytuacją, że kiedy próbujesz zrozumieć kod (w moim przypadku martwy), najpierw sprawdzasz dziennik kontroli wersji ze wszystkimi powiązanymi komunikatami zatwierdzania. Zamiast przeszukiwać dziennik, kolega od razu widzi, że ten plik jest bezużyteczny. Oszczędza to jego czas i ona / on wie, że ten plik został prawdopodobnie przywrócony na złe (a przynajmniej rodzi pytanie.

Problem z dodaniem komentarza do pliku, który powinien zostać usunięty, zamiast usunięcia go w kontroli źródła i umieszczenia tam wyjaśnienia, polega na założeniu, że jeśli programiści nie będą czytać komunikatów zatwierdzenia, z pewnością przeczytają komentarze w kodzie źródłowym.

Z perspektywy osoby trzeciej ta metodologia wydaje się być oparta na bardzo konserwatywnym podejściu do kontroli źródła.

„Co się stanie, jeśli usunę ten nieużywany plik, a następnie ktoś będzie go potrzebował?” ktoś może zapytać.

Używasz kontroli źródła. Cofnij zmianę lub lepiej porozmawiaj z osobą, która usunęła plik (komunikuj się).

„Co się stanie, jeśli usunę martwy plik, a następnie ktoś ponownie go użyje i wprowadzi zmiany?” ktoś inny może zapytać.

Ponownie używasz kontroli źródła. Dostaniesz konflikt scalania, który dana osoba musi rozwiązać. Odpowiedzią tutaj, podobnie jak w przypadku ostatniego pytania, jest komunikacja z członkami drużyny.

Jeśli naprawdę masz wątpliwości, że plik powinien zostać usunięty, komunikuj się przed usunięciem go z kontroli źródła. Być może dopiero niedawno przestał być używany, ale nadchodząca funkcja może tego wymagać. Nie wiesz tego, ale jeden z innych programistów może.

Jeśli należy go usunąć, usuń go. Wytnij tłuszcz z bazy kodu.

Jeśli zrobiłeś „oopsie” i rzeczywiście potrzebujesz pliku, pamiętaj, że używasz kontroli źródła, aby odzyskać plik.

Vincent Savard w komentarzu do pytania powiedział:

... Jeśli twoi koledzy nie czytają komunikatów zatwierdzania i wskrzeszają plik, który został prawidłowo usunięty i przejdzie przegląd kodu, na pewno coś jest nie tak w twoim zespole i jest to świetna okazja, aby nauczyć go lepiej.

To dobra rada. Przeglądy kodu powinny być tego rodzaju. Programiści muszą sprawdzać komunikaty zatwierdzania, gdy plik zostanie nieoczekiwanie zmieniony lub plik zostanie usunięty lub jego nazwa zostanie zmieniona.

Jeśli komunikaty zatwierdzania nie opowiadają historii, programiści muszą również pisać lepsze komunikaty zatwierdzania.

Obawa przed usunięciem kodu lub plików oznacza głębszy, systemowy problem z procesem:

• Brak dokładnych recenzji kodu
• Brak zrozumienia, jak działa kontrola źródła
• Brak komunikacji w zespole
• Słabe komunikaty zatwierdzania przez programistów

Są to problemy do rozwiązania, więc nie czujesz się, jakbyś rzucał kamieniami w szklany dom, gdy usuwasz kod lub pliki.

Tak, to zła praktyka.

Wyjaśnienie dotyczące usunięcia należy umieścić w komunikacie zatwierdzenia podczas zatwierdzania usunięcia plików.

Komentarze w plikach źródłowych powinny wyjaśniać kod, jak obecnie wygląda . Komunikaty zatwierdzania powinny wyjaśniać, dlaczego wprowadzono zmiany w zatwierdzeniu, więc historia zatwierdzeń w pliku wyjaśnia jego historię.

Pisanie komentarzy wyjaśniających zmiany bezpośrednio w samym źródle znacznie utrudni śledzenie kodu. Pomyśl o tym: jeśli komentujesz w pliku za każdym razem, gdy zmieniasz lub usuwasz kod, wkrótce pliki zostaną zalane komentarzami na temat historii zmian.

Moim zdaniem obie opcje nie są najlepszą praktyką , w przeciwieństwie do złej praktyki:

1. Dodanie komentarzy dodaje wartość tylko wtedy, gdy ktoś czyta te komentarze.
2. Samo usunięcie z VCS (z podaniem przyczyny w opisie zmiany) może mieć wpływ na krytyczne wyniki i wiele osób nie czyta opisu zmiany, szczególnie pod presją.

Moją ulubioną praktyką - głównie ze względu na to, że byłam gryziona kilka razy przez lata, pamiętając, że nie zawsze wiesz, gdzie lub przez kogo jest używany Twój kod - jest:

1. Dodaj komentarz stwierdzający, że jest to kandydat do usunięcia i wycofanie #warning lub podobne (większość języków ma jakiś taki mechanizm, np. W Javie , w najgorszym przypadku wystarczą printstwierdzenia lub podobne), najlepiej z pewnym przedziałem czasowym i z danymi kontaktowymi. Zaalarmuje to każdego, kto nadal używa kodu. Te ostrzeżenia są zwykle wewnątrz każdej funkcji lub klasy, jeśli takie rzeczy istnieją .
2. Po pewnym czasie zaktualizuj ostrzeżenie do standardowego zakresu plików #warning(niektóre osoby ignorują ostrzeżenia o wycofaniu, a niektóre łańcuchy narzędzi domyślnie nie wyświetlają takich ostrzeżeń).
3. Później zamień zakres pliku #warningna #errorlub równoważny - spowoduje to uszkodzenie kodu w czasie kompilacji, ale można, jeśli jest to absolutnie konieczne, usunąć, ale osoba go usuwająca nie może go zignorować. (Niektórzy programiści nie czytają ani nie reagują na żadne ostrzeżenia lub mają ich tak wiele, że nie są w stanie oddzielić ważnych).
4. Na koniec zaznacz plik (i) (w terminie lub po terminie) jako usunięte w VCS.
5. Jeśli usuwam cały pakiet / bibliotekę / etc na etapie ostrzegania, dodam plik README.txt lub README.html lub podobny z informacją o tym, kiedy i dlaczego planowane jest pozbycie się pakietu i pozostawienie tylko tego pliku, z zmień, aby powiedzieć, kiedy został usunięty, jako jedyna zawartość pakietu przez pewien czas po usunięciu reszty zawartości.

Zaletą tego podejścia jest to, że niektóre systemy kontroli wersji (CVS, PVCS itp.) Nie usuwają istniejącego pliku przy kasie, jeśli został on usunięty w VCS. Daje to również sumiennym programistom, tym, którzy naprawiają wszystkie swoje ostrzeżenia, mnóstwo czasu na rozwiązanie problemu lub odwołanie się od usunięcia. Zmusza to również pozostałych programistów do przynajmniej spojrzenia na powiadomienie o usunięciu i narzekania .

Zauważ, że #warning/ #errorjest mechanizmem C / C ++, który powoduje ostrzeżenie / błąd, a po nim dostarczony tekst, gdy kompilator napotka ten kod, java / java-doc ma @Deprecatedadnotację, aby wydać ostrzeżenie o użyciu i @deprecatedpodać pewne informacje itp. Istnieją przepisy na podobne działania w Pythonie i widziałem assertużywane do dostarczania podobnych informacji do #errorprawdziwego świata.

Tak, powiedziałbym, że to zła praktyka, ale nie dlatego, że jest w plikach, a nie w komunikacie zatwierdzenia. Problem polega na tym, że próbujesz dokonać zmiany bez komunikacji ze swoim zespołem.

Ilekroć wprowadzasz jakiekolwiek zmiany do pnia - dodając, usuwając lub modyfikując pliki w jakikolwiek sposób - powinieneś, zanim zwrócisz się do pnia, rozmawiać o tych zmianach bezpośrednio z członkiem (lub członkami zespołu), który byłby bezpośrednio posiadający wiedzę na ich temat (w zasadzie omawiajcie, co chcielibyście umieścić w tych nagłówkach bezpośrednio z kolegami z drużyny). Zapewni to, że (1) usuwany kod naprawdę musi zostać usunięty, a (2) Twój zespół będzie o wiele bardziej prawdopodobne, że dowie się, że kod został usunięty, a zatem nie spróbuje cofnąć usuniętych danych. Nie wspominając o tym, że otrzymasz także wykrywanie błędów i tym podobne dla dodanego kodu.

Oczywiście, kiedy zmieniasz duże rzeczy, powinieneś również umieścić je w komunikacie zatwierdzenia, ale ponieważ już to omawiałeś, nie musi to być źródło ważnych ogłoszeń. Odpowiedź Steve'a Barnesa dotyczy również kilku dobrych strategii, które można zastosować, jeśli potencjalna pula użytkowników dla twojego kodu jest zbyt duża (np. Użycie przestarzałych znaczników języka zamiast usuwania plików na początku).

Jeśli nie chcesz iść tak długo bez zobowiązania (tzn. Zmiana ma sens w przypadku kilku wersji), dobrym pomysłem jest utworzenie gałęzi z pnia i zatwierdzenie do gałęzi, a następnie scalenie gałęzi z powrotem w pień, gdy zmiana jest gotowa. W ten sposób VCS nadal tworzy kopię zapasową pliku, ale twoje zmiany w żaden sposób nie wpływają na łącze.

Powiązany problem związany z projektami opartymi na wielu platformach i konfiguracjach. Tylko dlatego, że ustalenie, że jest martwy, nie oznacza, że jest to właściwe określenie. Pamiętaj, że martwy w użyciu może nadal oznaczać szczątkową zależność, którą wciąż trzeba usunąć, a niektóre mogły zostać pominięte.

Więc (w projekcie C ++) dodałem

#error this is not as dead as I thought it was

I sprawdziłem to. Poczekaj, aż przejdzie przez odbudowę wszystkich normalnych platform i nikt nie narzeka. Gdyby ktoś go znalazł, łatwo byłoby usunąć jedną linię, zamiast oszołomić brakującym plikiem.

Zgadzam się co do zasady, że wspomniane komentarze duplikują funkcję, którą należy wykonać w systemie zarządzania wersjami . Ale możesz mieć konkretne powody, aby je uzupełnić. Brak znajomości narzędzia VCS nie jest jednym z nich.

Na kontinuum złych praktyk jest to dość niewielkie. Zrobię to czasami, gdy chcę sprawdzić oddział i zobaczyć stary kod. Może to ułatwić porównanie z kodem zastępczym. Zazwyczaj pojawia się komentarz do recenzji kodu „cruft”. Z małym wyjaśnieniem przechodzę przegląd kodu.

Ale ten kod nie powinien długo żyć. Generalnie usuwam na początku następnego sprintu.

Jeśli masz współpracowników, którzy nie czytają komunikatów zatwierdzania lub nie pytają cię, dlaczego zostawiłeś kod w projekcie, problem nie dotyczy złego stylu kodowania. Masz inny problem.

możesz również zmienić klasę i zmienić jej nazwę z prefiksem UNUSED_Classname, zanim wykonasz wyczyn. Następnie powinieneś bezpośrednio zatwierdzić operację usunięcia

• Krok 1: zmień nazwę klasy, dodaj swój komentarz, zatwierdź
• Krok 2: Usuń plik i zatwierdź

Jeśli jest to martwy kod, usuń go. Każdy tego potrzebuje, może wrócić, ale krok zmiany nazwy uniemożliwi bezpośrednie użycie go bez zastanowienia.

Naucz także ludzi, jak sprawdzać wszystkie komunikaty zatwierdzania pliku, niektórzy nawet nie wiedzą, że jest to możliwe.