Czy można wprowadzać zmiany w stylu kodowania w projekcie open source, który nie przestrzega najlepszych praktyk?

38

Ostatnio natknąłem się na wiele projektów Ruby (lub większość z nich to Ruby) na GitHub, które po sprawdzeniu za pomocą narzędzia do analizy kodu, takiego jak Rubocop , powodują wiele przestępstw .

Obecnie większość tych przestępstw obejmuje stosowanie podwójnych cudzysłowów zamiast pojedynczych cudzysłowów (gdy nie są interpolacjami), nieprzestrzegania reguły 2 spacji na poziom, przekraczania reguły długości 80 znaków lub używania {i }dla bloków wieloliniowych.

[Przewodnik] w stylu Ruby zaleca najlepsze praktyki, aby programiści Ruby w prawdziwym świecie mogli pisać kod, który może być obsługiwany przez innych programistów Ruby. ~ Źródło: Ruby Style Guide

Chociaż są małe i łatwe do naprawienia, czy należy zmienić styl kodowania projektu open source, naprawiając przestępstwa i składając wniosek o wycofanie? Przyjmuję do wiadomości, że niektóre projekty, takie jak Rails, nie akceptują zmian kosmetycznych, a niektóre są po prostu zbyt duże, aby „naprawić” naraz (na przykład Railsy generują ponad 80 000 przestępstw podczas uruchamiania Rubocop - niezależnie od tego, że mają swój własny mały zestaw kodowania konwencje, których należy przestrzegać podczas wnoszenia wkładu). W końcu Przewodnik po stylu Ruby istnieje nie bez powodu wraz z narzędziami takimi jak Rubocop.

Ludzie cenią sobie konsekwencję, więc wprowadzanie tego rodzaju zmian jest ogólnie rzeczą dobrą dla społeczności Ruby, prawda?

[Autor (y) Przewodnika po stylu Ruby] nie wymyślił wszystkich zasad znikąd - są one w większości oparte na mojej rozległej karierze zawodowej jako inżynier oprogramowania, opiniach i sugestiach członków społeczności Ruby i różnych cenione zasoby programistyczne Ruby, takie jak „Programowanie Ruby 1.9” i „Język programowania Ruby”. ~ Źródło: Ruby Style Guide

Czy przestrzeganie konwencji i najlepszych praktyk dotyczących kodowania społeczności nie zachęca do złych praktyk?

raf
źródło
3
Podobne pytanie: Czy powinienem refaktoryzować klasę F na podstawie kodu klimatycznego? , ale z większym naciskiem na architekturę.
amon
5
Wiele z tych problemów ze stylem brzmi całkiem nieźle.
JL235
4
@MathewFoscarini nie, pytają: „jeśli kupię pistolet radarowy, czy mogę zdecydować, jakie są lokalne prawa jazdy?”
Jon Hanna

Odpowiedzi:

65

Zapytaj opiekunów.

Styl kodowania jest dość subiektywną dyskusją, a zasady takie jak maksymalna długość linii wynosząca 80 znaków są dość subiektywne - podczas gdy ogólna zgoda powinna być taka, że ​​krótsze linie są lepsze do czytania, 80 może być zbyt restrykcyjne dla niektórych przy dzisiejszych rozmiarach ekranu i IDE.

Inne reguły można również celowo zignorować. Deweloper może na przykład rozważyć globalne stosowanie podwójnych cudzysłowów dla siebie i być gotowym zaakceptować „ryzyko” przypadkowej interpolacji i wyjątkowo niewielki wzrost czasu analizy.

Wielu opiekunów również nie lubi dużych zmian w stylu kodowania, ponieważ ich przegląd jest nudny i istnieje ryzyko, że może to spowodować błędy. Na przykład ciąg może być przełączony na pojedynczy cudzysłów, nawet jeśli zawiera celową interpolację i powinien był używać podwójnych cudzysłowów. Opiekunowie wolą robić porządki stylu podczas pracy nad tym kodem, aby mogli sprawdzić, czy zmiany stylu nie wprowadzają nowych błędów.

Johnnes
źródło
34
Wielu opiekunów również nie lubi dużych zmian, które nie są absolutnie konieczne, ponieważ mają tendencję do tworzenia konfliktów scalania, które również nie są absolutnie konieczne.
Jan Hudec
4
Utracisz funkcję adnotacji (która tworzy wiersz kodu) w kontroli źródła, jeśli wystąpi błąd, trudno jest obwiniać, gdzie został wprowadzony i śledzić, czy jest więcej tego rodzaju błędów.
peer
4
Ponadto - jeśli konwencje specyficzne dla projektu są spójne, można utworzyć konfigurację specyficzną dla projektu dla narzędzia sprawdzającego konwencje i zaoferować konfigurację konwencji jako żądanie ściągnięcia. Tak więc opiekunowie mogą sprawdzić swój projekt za pomocą konfiguracji. (Nie wiem, czy jest to możliwe przy użyciu narzędzia, którego użyłeś).
Peter Kofler
+1 w konfliktach scalania. Właśnie zaakceptowałem przeróbkę stylu kodowania i żałuję, że tego nie zrobiłem, ponieważ spowodowało to połączenie konfliktów z PR innych osób. Łatwo to rozwiązać, ale wolałbym to zrobić kawałek po kawałku
Daenyth
To IDE jest restrykcyjne, jeśli nie pozwala na pokazanie dwóch osobnych plików obok siebie, a nie wina kodu o krótkiej linii.
unperson325680,
48

Wygląda na to, że kierujesz się głównie szacunkiem dla autorytetu narzędzia rubocop i Przewodnika po stylu Ruby, którego opiekunowie mogą nie udostępniać. Mają już swój własny styl i są do niego przyzwyczajeni, więc każda zmiana wpłynęłaby na wszystkich pracujących nad projektem, a to dużo pracy, szczególnie jeśli projekt jest duży.

Rozważ motywacje opiekunów. Prawdopodobnie chcą, aby dołączyli do nich nowi ludzie, przesyłając poprawki błędów, raporty o błędach, działający kod i dobrze napisaną dokumentację. Jeśli pojawisz się i powiesz „Masz przestępstwa w rubocopie”, nie będą myśleć: „Och, dobrze, ktoś nowy, aby pomóc podzielić się obciążeniem”, pomyślą: „Kim jest ten facet? Dlaczego on nam mówi? co robić?".

Projekty open source są zwykle merytokracjami: zyskujesz szacunek w oparciu o jakość swojej pracy. Jeśli pojawisz się i robisz wspaniałe rzeczy, a następnie poruszasz swoje obawy związane ze stylem, chętniej słuchają, chociaż nadal mogą odmówić. Istnieje otwarte źródło mówiące: „Dyskusja jest tania, pokaż mi kod”.

Michael Shaw
źródło
1
Najlepsza odpowiedź. Musisz szanować wysiłki tych, którzy przybyli wcześniej, składając wnioski ściągania. Zmiana stylu projektu pod stopami wszystkich istniejących programistów, szczególnie tych, którzy mają wyższą „rangę” od ciebie w merytokracji, utrudnia im zapamiętanie nowych zasad, które wprowadzasz. Zaszkodziłoby to ich zdolności do utrzymania projektu takim, jakim były. Ponadto, jeśli byłeś już aktywny w rozwoju projektu, prawdopodobnie znasz przemyślenia opiekuna na temat zmian stylistycznych i najlepszy moment w cyklu życia projektu, aby je wprowadzić.
Chris Keele
1
Dokładnie. Na przykład projekt Linuksa, pomimo dość ścisłego przewodnika po stylach dla nowych fragmentów kodu, nie pozwoli ci po prostu przesłać dużego żądania naprawienia problemów ze stylem, ponieważ nie działa on ze scalaniem. Prosi nawet o zachowanie lokalnego stylu, nawet jeśli oznacza to, że jest niezgodny z przewodnikiem stylu projektu.
Miles Rout
25

Pragmatyzm nad dogmatem, zawsze. Przewodniki w stylu kodowania są szczególnie podstępną formą zła, która odwraca uwagę od kwestii architektonicznych w kierunku frywolnych bzdur, takich jak pojedyncze / podwójne cytowanie. Zadaj sobie pytanie: czy to naprawdę robi różnicę?

Mogą być do pewnego stopnia dobre, ale gdy tylko traktujesz ich z niemal religijnym zapałem, zaszedłeś za daleko. Są to wytyczne, sugestie, opinie, a NIE fakty.

Czy należy je zatem zignorować? Nie, warto korzystać z narzędzi, aby uzyskać ogólne pojęcie o tym, na co należy zwrócić uwagę, ale nie więcej.

To dziwne, jak często młodsze typy mylą opinię.

baweaver
źródło
11
Warto dodać, że zmiany formatowania zwykle powodują konflikty scalania, co jest bardzo dobrym powodem dla większości opiekunów, aby nienawidzili formatowania tylko ze względu na to.
Jan Hudec
13

Możesz wyciągnąć te zmiany tylko wtedy, gdy istnieje otwarty problem, aby naprawić formatowanie. W przeciwnym razie uruchom własną gałąź, a jeśli autor zauważy, że więcej osób korzysta z twojej gałęzi tylko dlatego, że jest bardziej czytelna. Połączą się w oddziale samodzielnie, ale bądź przygotowany na utrzymanie oddziału przez scalanie aktualizacji i ciągłe poprawianie formatowania.

Jeśli projekt nie jest dla ciebie wystarczająco ważny, aby utrzymać własny oddział, to nie warto go w ogóle porządkować.

Żądania ściągnięcia mogą być podejmowane osobiście przez autora. To nie jest mechanizm oferowania krytyki, a przeformatowanie całego kodu można uznać za krytykę.

Jeśli nie chcesz prowadzić własnego oddziału, ale chcesz wnieść wkład w projekt. Otwórz nowy numer i opisz, dlaczego bieżący format powoduje problemy, a następnie zaoferuj autorowi rozwiązanie problemu. Jeśli autor wyrazi zgodę, wówczas przypisuje ci problem, a ty masz teraz pozwolenie na wysłanie prośby.

Poruszyłeś temat, który, jak się zgadzam, jest powszechnym problemem w GitHub . Pomijając formatowanie, istnieje wiele projektów, które niepoprawnie używają adnotacji i powodują spustoszenie wielu IDE. Mogę wymyślić trzy bardzo popularne projekty, które nieprawidłowo używają przestarzałych flag, które propagują komunikaty ostrzegawcze w moim IDE. Wysłałem żądania ściągnięcia, aby je naprawić, ale autorzy nie używają tego samego IDE, więc żądania ściągania są ignorowane.

Rozgałęzianie, łączenie i utrwalanie wydaje się jedynym rozwiązaniem.

Reactgular
źródło
Czy Twoim zdaniem korzyści dla przyszłych autorów byłyby uzasadnioną „wymówką” w związku z żądaniem naprawienia wykroczeń? Na przykład, aby przyszli współtwórcy nie musieli się martwić przeglądaniem całej bazy kodu, aby uchwycić swój styl kodowania.
raf
@RafalChmiel Gdy autor zaakceptuje żądanie ściągnięcia, osoba, która je wysłała, zostaje dodana do repozytorium i publicznie wymieniona jako autor projektu i pozostaje na liście jako autor. Przeglądając żądanie ściągnięcia, autor będzie o tym pamiętać przy podejmowaniu decyzji o jego zaakceptowaniu. Pamiętaj o tym, wysyłając żądania ściągania. Rób rzeczy godne bycia współpracownikiem.
Reactgular
Chodziło mi o to, że korzyści przyszłych współpracowników poprzez ustalenie przestępstwa być ważny powód dla połączenia PR z tych poprawek? Dlaczego opiekun powinien łączyć taki PR? Może treść mojego pytania była trochę odległa.
raf
2
@RafalChmiel wszystko, co podajesz jako przyczynę, jest tylko Twoją opinią. Jeśli jest to obraźliwy kod, który wygląda jak byk, napisz swoje obawy w numerze. Jeśli autor broni się przed takim pociągnięciem, wytrzyj łzy chusteczką.
Reactgular
10

Zaczerpnięte z samej strony Rubocop (moje podkreślenie):

Jedna rzecz zawsze mnie niepokoiła jako programista Ruby - programiści Python mają świetne odniesienie do stylu programowania (PEP-8) i nigdy nie otrzymaliśmy oficjalnego przewodnika dokumentującego styl kodowania Ruby i najlepsze praktyki.

Proszę zrozumieć:

Nie ma oficjalnego przewodnika po stylu Ruby

Nie twierdzę, że przewodniki po stylu są złe. Ale nie tylko nie ma oficjalnego przewodnika, ale posiadanie przewodnika po stylu to wybór dokonywany na poziomie osobistym, projektu, zespołu lub firmy. Przewodniki po stylu są, mówiąc terminem psychologicznym, „normą społeczną w grupie”.

Co to dla ciebie znaczy? Cóż, jeśli nie jesteś uważany za członka grupy, oznacza to, że coś, co ty lub jakakolwiek inna strona internetowa mówi, jest bardzo mało prawdopodobne, aby mieć wpływ na grupę. Jeśli więc nie jesteś aktywnym i szanowanym współtwórcą tego konkretnego projektu, najprawdopodobniej Twoje sugestie zostaną zignorowane lub będą w najlepszym razie przypomnieniem poprzednich rozważań na temat posiadania przewodnika po stylu. W najgorszym przypadku będzie to traktowane jako zniewaga lub jako intruz lub rowerzysta wbijający nos tam, gdzie nie należy.

Nie możesz po prostu zaproponować Przewodnika po stylu?

W rzeczywistości wydaje się, że właśnie to chcesz robić: wierzysz w wartość przewodników po stylu, cenisz spójność i chcesz ewangelizować poświęcenie się jednolitym wytycznym stylu.

W porządku, o ile jesteś naprawdę pewien, że właśnie o to chodzi i co chcesz osiągnąć. Jeśli wierzysz w jakiś Przewodnik po stylu i wierzysz, że jest to Przewodnik po prawdziwych stylach, a przynajmniej lepiej niż cokolwiek, co praktykują ci bezbożni poganie, to też jest w porządku.

Ale tym, czego ludzie nie doceniają, jest mówienie, że ich zachowanie nie jest zgodne z nieoficjalnymi, niewiążącymi, w dużej mierze arbitralnymi przepisami, które pochodzą ze źródła, którego nie uważają za uprawniony autorytet. Kiedy to właśnie zamierzasz zrobić, lub jeśli tylko to jest postrzegane jako działanie, dostaniesz mniej „traktowania czerwonego dywanu”, a więcej „wściekłych tubylców z włóczniami i dużym garnkiem” leczenie.

BrianH
źródło
3

Aby w tym przypadku przedstawić alternatywną opinię, w wielu przypadkach taka zmiana byłaby pożądana. Projekty open source mają zwykle wielu autorów. Często nie ma „stylu kodowania”; styl jest po prostu tym, czego używa osoba, która napisała dany kod. Jeśli jeden plik został napisany przez inną osobę niż inna, styl może być również inny. Nawet w projektach, w których istnieje styl konsensusowy, chyba że regularnie sprawdzają ten styl, często nie jest on używany.

Typowym przykładem tego jest moje doświadczenie, gdy ktoś wnosi jakiś kod poprzez żądanie ściągania, które jest stosunkowo niskiej jakości pod względem stylu. Kod może jednak działać. Różni ludzie mają różne zdanie na ten temat. Niektóre osoby odmówią scalenia żądania ściągnięcia, chyba że styl jest dobry. Niektórym nie zależy, dopóki kod działa. Niektórzy ludzie wolą mieć dobry styl, ale nie chcą odstraszyć współpracowników za pomocą szeregu komentarzy „napraw tutaj białe znaki” (osobiście zawsze czuję się trochę winny, robiąc te komentarze, chociaż wiem, że są na lepsze dobra baza kodu, ponieważ wydaje się, że może odstraszyć autora).

Nie zakładaj więc od razu, że styl, który widzisz, to styl, którego chce projekt. W rzeczywistości można to prawdopodobnie uogólnić na przyczynianie się ogólnie do open source: nie zakładaj, że kod projektu open source jest kodem, którego chce .

Należy jednak pamiętać o niektórych rzeczach:

  • Niektórzy ludzie religijnie podchodzą do stylu. Jeśli stanie się jasne, że nie chcą się ruszyć, nie przejmuj się.

  • To ogromny problem z rowerami . Wszyscy i ich brat mają zdanie na temat tych rzeczy. Z tego powodu połączenie takiego żądania ściągnięcia może być trudne.

  • Połączenie takiego żądania ściągania może być również trudne, ponieważ bardzo szybko spowoduje konflikty scalania; w zasadzie za każdym razem, gdy jakakolwiek część bazy kodu, którą zmodyfikowałeś, zmienia się, nawet jeśli jest to banalna metoda.

Trzymałbym się podejścia „najpierw zapytaj”. Jeśli są na to otwarci i jesteś gotów trzymać się żądania zakończenia do końca, to idź do niego.

asmeurer
źródło
2

Kiedyś bardzo lubiłem mieć dobry przewodnik po stylu, ale biorąc pod uwagę stan rzeczy w Ruby, „przeniosłem się”.

Zasadniczo żyję z tym, nad czym pracuję i w inny sposób przestrzegam ogólnych konwencji, których nauczyłem się z wielu miejsc pracy.

Dla Ruby, który jest moim wybranym językiem, podzieliłem również (w mojej głowie) styl na powszechnie akceptowane, ogólnie akceptowane, moje preferencje i najlepsze praktyki. Rzeczy powszechnie akceptowane Mógłbym przesłać zmianę w ramach prośby o zmianę dotyczącą problemu lub żądania oddziału.

Przykłady każdego stylu (moim zdaniem):

Powszechnie akceptowane dla Ruby:

  • spacje nie tabulatory
  • dwa wcięcia spacji
  • method_names_use_underscores
  • Stałe zaczynają się od Caps.

Generalnie zaakceptowane:

  • Nie używaj nawiasów, jeśli nie jest to konieczne
  • Użyj { }dla bloków jednokreskowych i bloków do endwieloliniowych.
  • CONSTANTS_ARE_IN_ALL_CAPS
  • Użyj wyrażeń predykatywnych ( cond ? true : false), if thenjeśli wyrażenie pasuje do 1 wiersza.

Preferencje osobiste:

  • Długość linii 120 linii znaków
  • wheninstrukcje case są wcięte 2 od instrukcji case.
  • Używaj pojedynczych cudzysłowów, chyba że podwójne są potrzebne.

Brak umowy:

  • Oświadczenia dotyczące spraw
  • Długość linii

Najlepsze praktyki:

  • małe metody, <= 5 linii, jeśli to możliwe.
  • małe klasy, <= 100 linii, jeśli to możliwe.
  • Unikaj stałych
  • Kod zawiera testy

Wreszcie, tak jak inni szczegółowo, powinieneś najpierw zapytać. Ostatecznie kluczem do stylu jest komunikacja między programistami i wrażliwość na innych. Na przykład, jeśli chcę zmienić styl w projekcie open source, często najpierw wykonuję żądanie ściągnięcia jednej lub dwóch rzeczywistych funkcji lub poprawek błędów. Gdy opiekun mnie zna i widzi, że już przyczyniły, wtedy mógłbym zasugerować zmiany stylu. Sugeruję tylko je. Na przykład „Robiąc inną funkcję w projekcie x, zauważyłem, że masz kilka wcięć w spacji w kilku plikach i zastanawiałem się, czy mógłbym je zmienić na 2?”

Michael Durrant
źródło
1

Chociaż są one małe i łatwe do naprawienia, czy uważasz, że można zmienić styl kodowania projektu open source, naprawiając przestępstwa i wysyłając żądanie ściągnięcia?

Krótko mówiąc, nie!

Oczywiście zakładam tutaj, że to tylko kwestia stylu i nie są to prawdziwe błędy - żądania ściągania dla tych drugich zawsze byłyby rozsądne (IMO). Zakładam również, że jesteś obcy dla projektu i brak kontaktu z osobami, które już go utrzymują.

Jednak w każdym języku zawsze są ludzie, którzy mają swoje preferencje, które różnią się od przewodnika po stylu, na lepsze lub gorsze, i próbują wymusić te zmiany stylu u osób, których nie znasz , i projektu, którego nie jesteś częścią o jeżeli nic innego nie mógł trafia jak trochę niegrzeczny. W końcu - co osiągasz (w rzeczywistości), jeśli prośba została zaakceptowana? Najprawdopodobniej gdyby członkowie jednego projektu chcieli zmienić styl na coś innego, zrobiliby to już - i wszystko, co robilibyście z tym żądaniem, to narzucenie im stylu, który niekoniecznie działa lepiej dla obecnych członków.

Zmienia się to nieznacznie, jeśli chcesz przyczynić się do projektu na inne sposoby niż robienie „poprawek” stylu. Powiedziałbym, że jeśli chcesz otworzyć dialog z opiekunami na temat tego, jak chcesz pracować nad projektem, ale uważasz, że styl niestandardowy jest trudny, to w porządku. Ale tak naprawdę nie chciałbym po prostu ślepo tworzyć zleceń ściągania dla wielu projektów, które zawierają tylko zmiany stylu!

berry120
źródło
1

DOWOLNA zmiana może wprowadzić błędy, a nawet zmienić formatowanie typograficzne kodu.
Dlatego nie należy wprowadzać żadnych zmian w kodzie, chyba że jest to poprawne uzasadnienie biznesowe, a „ale kod nie wygląda ładnie” lub „kod nie spełnia standardów kodowania” nie jest prawidłowym uzasadnieniem biznesowym. Ryzyko jest po prostu zbyt duże.
Teraz, jeśli i tak dokonujesz poważnych zmian w pliku źródłowym, ciągnięcie całego pliku zgodnie ze standardami może być do zaakceptowania, ale najprawdopodobniej będziesz wprowadzać małe zmiany, w takim przypadku prawie zawsze lepiej jest zachować własne zmiany w zgodne z istniejącym kodem, mimo że istniejący kod nie spełnia standardów kodowania.
Do licha, kod może być wynikiem generatora kodu, a czasem może zostać zregenerowany. Generatory kodu są znane z produkowania brzydkiego kodu ...

jwenting
źródło
1

Mam kilka umiarkowanie udanych projektów .NET i miałem kilka PR-ów od ludzi, którzy, jak się wydaje, przeszli przez kod z ReSharper i StyleCop i „naprawili” wiele rzeczy. Nie akceptuję tych PR z kilku powodów:

  • Chociaż wiele zmian jest na lepsze, niektóre negatywnie wpływają na kod w jakiś sposób, zwykle na wydajność, a wybieranie dobrych części jest zbyt ciężką pracą.
  • Nawet jeśli wszystkie zmiany były albo łagodne, albo nawet korzystne, każda zmiana w każdym pliku w każdym zatwierdzeniu musi zostać przejrzana, i znowu, to po prostu trwa zbyt długo.

To powiedziawszy, jeśli ktoś chciałby dodać jakieś lepsze sprawdzanie błędów lub komentarze doktora, przyjąłbym ten PR w mgnieniu oka.

Mark Rendle
źródło
-1

Musisz dowiedzieć się, czy opiekunowie uważają te naruszenia stylu kodowania za wadę, czy też projekt ma inny standard stylu kodowania.

Jeśli ma on inny standard, możesz zaoferować pomoc w jego udokumentowaniu lub sformalizowaniu. Wtedy może się okazać, że istnieją pewne naruszenia tego stylu i można je naprawić.

Tim
źródło
wydaje się, że nie oferuje nic cennego w porównaniu z inną odpowiedzią, która została opublikowana kilka godzin przed tą odpowiedzią
komara