MediaWiki

Gerrit/Hodnocení kódu

< Gerrit
This page is a translated version of the page Gerrit/Code review and the translation is 100% complete.
Languages:

Toto je průvodce revizí a slučováním příspěvků do úložišť kódu Wikimedie, napsaný primárně pro vývojáře "provádějící" recenze kódu.

Cíle

  • Poskytujte rychlé recenze, abyste se vyhnuli bitrotu a pocitům opuštěnosti. Rychlá zpětná vazba povzbuzuje vývojáře, aby nadále přispívali, a pomáhá tak rozšířit naši základnu přispěvatelů.
  • Buď milý. Přispěné záplaty jsou dárky. Recenze ovlivňují vnímání dobrovolníka o celém projektu.
  • Z dlouhodobého hlediska povzbuzujte přispěvatele kódu, aby se také stali recenzenty.

Hledání oprav ke kontrole

 
Někteří vývojáři vás mohou požádat o kontrolu jejich kódu prostřednictvím dronu.

Je zde spousta kódu ke kontrole. Jak rozdělit úkol na lépe zvládnutelné části?

Existuje několik základních vzorů:

Na požádání

Contributors can request code review in Gerrit by adding a reviewer and clicking "Add to attention set". This is a helpful way to indicate to a user that you wish them to do a review.

 
In the Gerrit UI patch contributors can request reviewer by adding their user name and clicking "Add to attention set"

Reviewers should therefore bookmark and monitor requests for code review in this form using the Gerrit URL query has:attention.

When a patch has your attention you can either:

  • Provide a review for the patch as requested (which will move the attention back to the patch contributor) OR
  • If you are not the right person to review the patch, comment and/or suggest alternative reviewers using the "add to attention set" feature. You can use the Maintainers page to identify alternative reviewers.

Podle projektu

Pokud jste správce, zvažte nastavení e-mailových upozornění na nové sady patchů ve vašich projektech (úložištích) prostřednictvím "Watched Projects" v Gerritu. Případně se můžete přidat do Gerrit Reviewer Bot, který vás automaticky přidá jako recenzenta do každé nové sady patchů.

  • Identifikujte hlavní části práce nebo sady souvisejících revizí. Dobrým způsobem, jak toho dosáhnout, může být rychlý chronologický přehled. Nebo si můžete vybrat úložiště s mnoha otevřenými sadami změn.
  • Otevřete všechny stránky changesetů jako karty v okně prohlížeče. Otevřete příslušné soubory v textovém editoru.
  • Zkontrolujte nové nebo neznámé soubory jako celek. Vyberte sadu změn s hlavní změnou v příslušném souboru a zkontrolujte ji.

Podle autora

  • Vyberte autora s (mnoha) otevřenými sadami změn, načtěte jej do Gerritu.
  • Procházejte revize chronologicky nebo postupujte po jednom tématu/úložišti.
  • Tato metoda umožňuje recenzentovi poznat jednotlivé vývojáře: Jejich dovednosti, chyby a zájmy. Práce má smysl pro pokrok a kontinuitu.

Pokud vás již někdo přidal jako potenciálního recenzenta a víte, že patch nebudete recenzovat, odeberte se ze seznamu recenzentů.[1]

Noví přispěvatelé do našich projektů

Do své nabídky můžete přidat (některý z) níže uvedených dotazů úpravou uživatelských preferencí. "Nový přispěvatel" je definován jako osoba, která přispěla celkem pěti nebo méně sadami změn.

Chronologické (a obráceně chronologické)

  • Začněte na nejstarší otevřené sadě změn, kontrolujte, dokud nedokončíte frontu. Alternativně začněte od poslední revize a čtěte postupně každý rozdíl, dokud nedosáhnete konce. Tento přístup je vhodný pro menší revize, ale vyžaduje neustálé přepínání mezi projekty a jejich kontexty.

Kontroly checklistu

Je to chtěné

  • Úplně první otázkou je, zda je příspěvek dobrý nápad. Pokud příspěvek není užitečný nebo neodpovídá směru a rozsahu projektu, vysvětlete a poskytněte zpětnou vazbu ohledně lepších nápadů.[2]

Obecné

  • Přidaný kód by měl fungovat tak, jak je inzerováno, to znamená, že by měly být opraveny všechny chyby nalezené v kódu. (Ale buďte opatrní, abyste neobvinili současného vývojáře z kódu napsaného předchozím vývojářem.)
  • Zachovejte zpětnou kompatibilitu pro stabilní rozhraní, pokud je to relativně jednoduché.
  • Pokud je nutná zásadní změna za účelem provedení významných vylepšení, ujistěte se, že jsou dodržovány zásady stabilního rozhraní.
  • Přečtěte si příslušné zprávy o chybách nebo dokumentaci.
  • Seznamte se se všemi relevantními technickými problémy. Přečtěte si příslušné specifikace nebo části návodu.

Výkon

  • Kód, který je v požadavku spuštěn mnohokrát, nebo kód, který se spouští při spuštění, by měl být zkontrolován z hlediska výkonu (např. členem Wikimedia Performance Team). Podezřelý kód může být potřeba otestovat.
  • Jakýkoli webový kód, který je velmi neefektivní (z hlediska času, paměti, počtu dotazů atd.), by měl být označen pro opravu (např. vytvořením úkolu pro Performance Team z Phabricatoru).
  • Změny schématu databáze nebo změny dotazů s vysokým provozem by měl zkontrolovat expert na databáze. (Odpovídající úkol Phabricator by měl mít přiřazenou značku značku "schema-change".)

Vývoj

  • Zlepší nebo zhorší tato změna uživatelský dojem pro koncové uživatele? Pokud to má vliv na uživatelskou zkušenost nebo vizuální design, zvažte konzultaci #wikimedia-design připojit se nebo vývojový mailing list nebo jednoho ze správců designu.

Styl

  • Ujistěte se, že jsou dodrženy konvence stylu kódování, jako jsou mezery, délka řádku, umístění složených závorek atd.

Čitelnost

  • Funkce by měly dělat to, co říkají jejich názvy. Výběr správného slovesa je zásadní, funkce get*() by měla něco získat, funkce set*() by měla něco nastavit.
  • Názvy proměnných by měly používat angličtinu. Pokud je to možné, nepoužívejte zkratky.
  • Preferují se komentáře k funkcím.
  • Příliš komplikovaný kód by měl být zjednodušen. To obvykle znamená prodloužen. Například:
    • Ternární operátory (?:) bude možná nutné nahradit if/else.
    • Dlouhé výrazy může být nutné rozdělit do několika příkazů.
    • Chytré použití přednosti operátorů, vyhodnocení zkratek, výrazy přiřazení atd. může být nutné přepsat.
  • Je třeba se vyhnout duplicitě, ať už v rámci souborů nebo mezi soubory.
    • Je to špatné pro čitelnost, protože je nutné hrát "zjistit rozdíl", abyste zjistili, co se změnilo. Čtení mnoha téměř kopií nějakého kódu nutně trvá déle než čtení jediné abstrakce.
    • Je to špatné pro udržovatelnost, protože pokud je v zkopírovaném kódu nalezena chyba (nebo chybějící funkce), všechny její výskyty musí být opraveny.
    • Někteří noví vývojáři mohou zkopírovat velké části kódu z jiných rozšíření nebo z jádra a změnit v něm některé drobné detaily. Pokud se zdá, že vývojář píše kód, který je pro jeho úroveň zkušeností "příliš náročný", zkuste grepovat základnu kódu pro některé fragmenty kódu, abyste identifikovali zdroj. Veďte vývojáře k přepsání nebo refaktorizaci.
    • Používání zkratek může být kontraproduktivní, protože množství času stráveného vymýšlením zkratky a ověřováním, zda funguje, bylo možné strávit úplným zapsáním původní myšlenky.

Zabezpečení

  • Recenzent by si měl přečíst a porozumět bezpečnostnímu průvodci a měl by si být vědom bezpečnostních problémů, které jsou zde probírány.
  • Neměla by existovat nejvzdálenější možnost spuštění libovolného kódu na straně serveru. To znamená, že na preg_replace() by nemělo být žádné eval() nebo create_function() a žádný modifikátor /e.
  • Zkontrolujte problémy s vkládáním textového protokolu (XSS, SQL injection atd.) Trvejte na výstupu na výstupní straně.
  • Zkontrolujte všechny akce zápisu pro CSRF (falšování požadavků napříč weby).
  • Dejte si pozor na speciální vstupní body, které mohou obcházet bezpečnostní ustanovení v WebStart.php.
  • Dejte si pozor na zbytečnou duplikaci základních funkcí MW důležitých pro zabezpečení, jako je použití $_REQUEST místo $wgRequest nebo escapování SQL pomocí addslashes() místo $dbr->addQuotes().
  • Pouze pokud pracujete na prastarém kódu: Zkontrolujte problémy s register_globals, zejména klasické svévolné chyby zabezpečení. (Register Globals byl odstraněn v PHP 5.4.0 a MediaWiki ≥1.27 vyžaduje PHP ≥5.5.9.)
  • V případě pochybností zvažte kontaktování Wikimedia Security Team.

Architektura

  • Jména, která jsou ve sdíleném (globálním) jmenném prostoru, by měla mít předponu (nebo jinak specifickou pro dané rozšíření), aby se předešlo konfliktům mezi rozšířeními. To zahrnuje:
    • Globální proměnné
    • Globální funkce
    • Jména tříd
    • Jména zpráv
    • Jména tabulky
  • Cílem modularity je oddělit zájmy. Moduly by na sobě neměly záviset neočekávaným nebo složitým způsobem. Rozhraní mezi moduly by měla být co nejjednodušší, aniž by se uchylovalo k duplikaci kódu.
  • Zkontrolujte podle Principů architektury.

Logika

  • Ukažte na zkratky a požádejte vývojáře, aby odvedl správnou práci.

Dokončení recenzi

  Varování: Nepoužívejte prosím "Verified +2". Jedná se o "silovou fúzi", která prochází testy. Viz také Gerrit/Privilege_policy#Merging_without_review

Poskytování pozitivní zpětné vazby

  • Pokud chcete pomoci s revizí kódu, ale necítíte se pohodově (zatím) s konečným rozhodnutím, můžete použít Code-Review +1 v Gerrit a uvést, zda jste kód "ověřili" a nebo "zkontrolovali".
  • Pokud je revize dobrá a prošla všemi výše uvedenými testy, označte ji v Gerritu Code-Review +2. Pokud vás něčí práce obzvlášť zaujala, řekněte to v komentáři. Když označíte závazek Code-Review +2, říkáte:
    • Zkontroloval jsem tento kód a
    • kód dává smysl a
    • kód funguje a dělá něco, co dělat chceme a
    • kód nedělá nic, co nechceme, aby dělal, a
    • kód se řídí našimi pokyny pro vývoj a
    • kód bude dávat smysl i za pět let.

Negativní zpětná vazba

  • Pokud je revize triviální, nefunkční a nemá žádnou zjevnou hodnotu, označte odevzdání jako "Code-Review -2"
  • Pokud má revize problémy, ale také má nějakou užitečnost nebo vykazuje známky toho, že směřuje správným směrem, označte ji Code-Review -1 a přidejte komentář vysvětlující, co je špatně a jak to opravit. Nikdy neoznačujte něco Code-Review -1 bez přidání komentáře. Pokud někdo označí váš kód Code-Review -1, znamená to, že váš kód je dobrý, ale potřebuje vylepšení.

Musíte zvážit náklady a přínosy každého postupu. Pokud změnu zcela odmítnete (Code-Review -2), tato změna bude ztracena a vývojáře to může odradit. Pokud chybu tolerujete, utrpí konečný produkt. Pokud to opravíte sami, necháte se vyvést z míry a možná nutíte vývojáře věřit, že je přijatelné odeslat nekvalitní kód a pak nechat někoho jiného, aby se staral o podrobnosti.

Obecné pokyny pro styl komentářů, zejména při poskytování negativní zpětné vazby:

  1. Zaměřte své komentáře na kodex a jakékoli objektivně pozorované chování, nikoli na motivaci. Například neuvádějte ani nenaznačujte domněnky o motivačních faktorech, jako je to, zda byl vývojář příliš líný nebo nezkušený, aby dělal věci správně. Místo požadavků pokládejte otázky, abyste podpořili technickou diskusi: "Co si myslíte o...?" "Uvažoval jsi...?" "Můžeš to vysvětlit...?"[3]
  2. Buďte empatičtí a laskaví. Uvědomte si, že si vývojář dal se svým nápadem pravděpodobně hodně práce, a poděkujte mu za jeho příspěvek, pokud to cítíte dobře a upřímně. "Proč jsi prostě..." poskytuje úsudek a staví lidi do defenzívy.[3] Buďte pozitivní.
  3. Dejte jim vědět, kde se mohou proti vašemu rozhodnutí odvolat. Pozvěte je například k diskusi o problému na mail:wikitech-l nebo na IRC.
  4. Buďte jasní. Necukrujte věci natolik, aby bylo hlavní poselství zastřeno.
  5. A co je nejdůležitější, poskytněte zpětnou vazbu rychle. Nenechávejte negativní zpětnou vazbu jen na někoho jiného nebo nedoufejte, že nebude dostatečně vytrvalý, aby byl jeho příspěvek přijat.

Kontakty

Během kontroly můžete mít nějaké otázky nebo problémy. Nebojte se! Můžeme se pokusit vám pomoci.

Máte-li dotazy týkající se Wikimedia Gerrit a kontroly kódu nebo konkrétních oprav, podívejte se na seznam IRC kanálů a vyberte si relevantní. Další platformy naleznete také na stránce Komunikace.

Pokud máte například otázky týkající se záplat MediaWiki, neváhejte se připojit k #mediawiki připojit se.

Pokud máte problémy s jenkins-bot, neváhejte laskavě kontaktovat #wikimedia-releng připojit se.

Související odkazy

Poznámky pod čarou

  1. ↑ Nemo_bis: "Počet recenzentů"
  2. ↑ Sage Sharp: Recenze Gentle Art of Patch (jemné umění záplatovat)
  3. ↑ 3.0 3.1 Prior, D., "Cultivating a Code Review Culture" at RailsConf 2015
Retrieved from "https://www.mediawiki.org/w/index.php?title=Gerrit/Code_review/cs&oldid=6810332"