Fórum Root.cz
Hlavní témata => Vývoj => Téma založeno: B.H.L. 05. 04. 2026, 07:35:48
-
Ahoj.
Zejména (ale nejen) v souvislosti s nástupem LLM generátorů kódu mi vrtá hlavou, jak přistupovat ke code review. Jak je děláte u Vás?
Je to jenom formalita, kdy to bezmyšlenkovitě odbouchnete?
Procházíte každý řádek změněného kódu a zamýšlíte se nad tím, jako byste to psali sami?
V našem týmu se to bere spíš jako formalita vynucovaná architekty, takže si to většinou bez čtení schválíme a jdem dál. Je to norma?
Máte nějaký metody jak si code review zefektivnit či zrychlit?
-
Procházím to a hledám v tom principiální chyby nebo věci, které by mohly ohrozit výkon aplikace.
-
Co si vsimam, tak ludia co robia cez LLM si nechavaju aj code review robit cez LLM a po tyzdni sa do kodu uz vobec nepozru (a to nazyvaju sami seba programatormi).
-
U LLM, ale i non-LLM kódu je potřeba hlavně pochopit, jak daný commit řeší zadaný problém. Dělá to, co má? Nedělá to něco navíc, co nebylo v zadání (pochopil pisatel kódu zadání, bylo jasné, co má dělat)? Nedělá to moc složitě (z výpočetního hlediska, struktury)? Drží to guidelines pro psaní kódu? Je kód srozumitelný, dobře okomentovaný, používá smysluplné identifikátory?
Některý commit lze schválit "formálně", když člověk na první dobrou vidí, že je vše v pořádku. Jindy je třeba se na to podívat pořádně. V dnešní době by ale každopádně měl první kolo code review dělat nějaký LLM, odhalí toho docela dost a rychle.
-
..mňa by skôr zaujímalo, či vznikajú aj "názorové konflikty" medzi programátormi, ktorí CodeReview vykonávajú. V praxi čo sledujem, CodeReview vykonávajú aspoň viac než dvaja programátori. Nebavíme sa o malých commitoch. Vždy ma hlavné slovo jeden z programátorov ? Leader ? Máte aj takú skúsenosť, že CedeReview programátori boli menej skúsení od Vás a prišlo k "ťažkej" výmene názorov ?
-
Procházím to a hledám v tom principiální chyby nebo věci, které by mohly ohrozit výkon aplikace.
Takže neřešíte třeba až tak úplně business logiku, jenom jestli tam nejsou nějaký úplně strašný a nebezpečný prasárny?
Co si vsimam, tak ludia co robia cez LLM si nechavaju aj code review robit cez LLM a po tyzdni sa do kodu uz vobec nepozru (a to nazyvaju sami seba programatormi).
Což je ovšem naprosto pochopitelný - jenom kontrolovat cizí kód je nevděčná práce a troufnu si tvrdit, že přinejlepším vědomí a svědomí málokdo dokáže jenom čtením porozumět cizímu kódu tak dobře, jako by ho psal sám.
Ale u části projektů vlastně na kvalitě kódu nesejde - Hlavně že to dělá, co to dělat má.
U LLM, ale i non-LLM kódu je potřeba hlavně pochopit, jak daný commit řeší zadaný problém. Dělá to, co má? Nedělá to něco navíc, co nebylo v zadání (pochopil pisatel kódu zadání, bylo jasné, co má dělat)? Nedělá to moc složitě (z výpočetního hlediska, struktury)? Drží to guidelines pro psaní kódu? Je kód srozumitelný, dobře okomentovaný, používá smysluplné identifikátory?
To zní, jakože na tom review musíte strávit snad stejně času jako původní řešitel, ne?
Některý commit lze schválit "formálně", když člověk na první dobrou vidí, že je vše v pořádku.
Jak tohle jste schopen posoudit na první dobrou? Nebo to je myšleno spíš v kontextu triviálních změn?
V dnešní době by ale každopádně měl první kolo code review dělat nějaký LLM, odhalí toho docela dost a rychle.
JJ, na tohle jsou ty plechovky super. U vygenerovanýho kódu jim pořád úplně nevěřím, ale review je možnost napáchat škody minimální, ale většinou něco odchytí.
-
U LLM, ale i non-LLM kódu je potřeba hlavně pochopit, jak daný commit řeší zadaný problém. Dělá to, co má? Nedělá to něco navíc, co nebylo v zadání (pochopil pisatel kódu zadání, bylo jasné, co má dělat)? Nedělá to moc složitě (z výpočetního hlediska, struktury)? Drží to guidelines pro psaní kódu? Je kód srozumitelný, dobře okomentovaný, používá smysluplné identifikátory?
To zní, jakože na tom review musíte strávit snad stejně času jako původní řešitel, ne?
Vůbec ne. U review nemusím vymýšlet, jak to celé poskládat, nemusím si projít těch 10 slepých cestiček, a tak dál. Pokud kód následuje mě známé vzorce, tak typicky nepřemýšlím řádek po řádku, ale po blocích. Podobně jako text nečtu slovo po slově, ale jako větu (dyslektici prominou). Například ohledně zmiňované výpočetní složitosti typicky budu chytat jen vnořené smyčky, častý opakovaný un-boxing při práci s jednoduchými typy, a takové věci. Ve většině případů ale nebudu řešit, jestli tohle je o 10% rychlejší, nebo pomalejší výpočet.
Některý commit lze schválit "formálně", když člověk na první dobrou vidí, že je vše v pořádku.
Jak tohle jste schopen posoudit na první dobrou? Nebo to je myšleno spíš v kontextu triviálních změn?
Za mě tohle jsou triviální změny (ale definuj triviálnost :D), nebo reviews, kde já jsem jen pro formální approval ("jo, může se použít tahle naše deprecated věc, protože to je jen přesun funkcionality z jedné třídy do druhé a existuje ticket na odstranění").
V dnešní době by ale každopádně měl první kolo code review dělat nějaký LLM, odhalí toho docela dost a rychle.
JJ, na tohle jsou ty plechovky super. U vygenerovanýho kódu jim pořád úplně nevěřím, ale review je možnost napáchat škody minimální, ale většinou něco odchytí.
+1, a z mojí zkušenosti mají větší šanci si všimnout chybějící změny, než člověk. Například přidal se nový volitelný field, to zahrnuje změnu 5 různých DTO, ale změnily se jen 4. Testy nepadají, protože se to taky přidalo jen do 4 testů z 5.
-
Co si vsimam, tak ludia co robia cez LLM si nechavaju aj code review robit cez LLM a po tyzdni sa do kodu uz vobec nepozru (a to nazyvaju sami seba programatormi).
Nevím, u nás děláme skoro jenom pomocí LLM, na code-review máme komplexní skills, ty rutinně používáme, ale stejně děláme code-review dost pečlivě i ručně. Takže to záleží. Není to žádné pravidlo. Vždycky platilo, že jsou lidé lepší a horší a bude to platit i dál. AI to neoslabuje, spíš naopak zesiluje.
..mňa by skôr zaujímalo, či vznikajú aj "názorové konflikty" medzi programátormi [...] Máte aj takú skúsenosť, že CedeReview programátori boli menej skúsení od Vás a prišlo k "ťažkej" výmene názorov ?
Naprosto. Za mě code-review velmi často totálně rozbíjí týmovou morálku. K tomu, aby to tak nebylo je třeba potkat fakt rozumné a neagresivní lidi. A takových je bohužel celkem málo.
Takže neřešíte třeba až tak úplně business logiku, jenom jestli tam nejsou nějaký úplně strašný a nebezpečný prasárny?
Záleží, na jaké úrovni myslíte. Na té nejnižší samozřejmě ano nebo pokud je tam nějaký zjevný problém. Na té vyšší ne. Napsat kód je 30 % práce vývojáře. Takže kdybych to měl řešit komplexně, vlastně budu tu práci dělat z velké části podruhé.
U LLM, ale i non-LLM kódu je potřeba hlavně pochopit, jak daný commit řeší zadaný problém. Dělá to, co má? Nedělá to něco navíc, co nebylo v zadání [...]
To zní, jakože na tom review musíte strávit snad stejně času jako původní řešitel, ne?
To má hlavně dělat ten, kdo tu AI vede a zadává ji práci.
Některý commit lze schválit "formálně", když člověk na první dobrou vidí, že je vše v pořádku.
Jak tohle jste schopen posoudit na první dobrou? Nebo to je myšleno spíš v kontextu triviálních změn?
Za mě třeba přesně tak.
-
Ahoj.
Zejména (ale nejen) v souvislosti s nástupem LLM generátorů kódu mi vrtá hlavou, jak přistupovat ke code review. Jak je děláte u Vás?
Je to jenom formalita, kdy to bezmyšlenkovitě odbouchnete?
Procházíte každý řádek změněného kódu a zamýšlíte se nad tím, jako byste to psali sami?
V našem týmu se to bere spíš jako formalita vynucovaná architekty, takže si to většinou bez čtení schválíme a jdem dál. Je to norma?
Máte nějaký metody jak si code review zefektivnit či zrychlit?
Záleží, k čemu se code review dělá. Jestli je to "jen" kvůli zajištění nějaké maintainability nebo i proto, aby se junioři něco nového naučili. U nás se snažím už jen o to první, protože předávání informací k juniorům přestalo (i "díky" LLM) být funkční a ani to asi není vyžadované. Takže: zapnuté všechny možné lintery s docela přísnými pravidly, definované agent.md s poměrně přesnou speckou, jak má kód vypadat, skills pro vývojáře, první nástřel kontroly je přes LLM a potom nastupuje ruční review, které je hodně časově náročná a vlastně i nevděčná činnost.
-
Jeste je zajimave se na to podivat z jineho pohledu.
Delate code review pred merge nebo az po nem?
Protoze tzv. gatekeeping code reviews ktere nedovoluji neco mergnout dokud nebude approvnuto jsou trochu dvojsecna zbran (jako vsechno... .)
1. Vede to na to, ze lidem se nechce a zacnou delat formalni approvaly.. nebo approvujou podle toho od koho je MR...
2. Vede to na to, ze si clovek rekne ze to po nem nekdo jeste zkontroluje takze je to sdilena odpovednost a treba nedava takovej pozor... ("Stejne mi to na poprvy vratej s poznamkama tak co bych se s tim drbal")
3. Vede to na to, ze se tolik netestuje a automatizace je na druhe koleji...
4. hrozne to zpomaluje flow... (mam treba rozdelany 3 pull requesty zaroven a tlacim je prez code review 3 dny nez si lidi udelaji cas na review zapracuju pozadovany zmeny apod.. )
Pokud naopak mam takovej test suite, ze tomu verim a mam v pipeline lintery a vsechny mozny udelatka co kdyz jsou spravne nastaveny tak spravne shodi build... A pak mam spravne nastavenej deployment a observability stack kterej sam zareaguje treba uz pri nasazeni na 10% podu ze nejaka metrika vykazuje anomalie a udela automatickej rollback... Tak muzu mit duveru v to, ze to proste pustim az na prod. A pri prusvihu mam zase tooling abych mohl rychle jit zpet.. nebo rychle zafixovat...
Zalezi na tom... pokud mas mission critical systemy (treba vesmir a letectvi) nebo dokonce life-critical (skoro cokoliv ve zdravotnictvi) Tak se to musi hodne zvazit treba ten cas a energii do toho dat... ale jinde muze byt vyznamne levnejsi to proste "poslat"
A pak to "ex post" code review muze naopak mit velky prinos.. udela se jednou za cas meeting kde se posedi a dohromady se probere co jde spravnym smerem a co jsou veci ktery by se radsi meli opravit... a vysvetli se proc a treba se to muze zdokumentovat pokud jsou v guidelines nejaky diry...
U me na tom PoC co rozebiram v jinem tematu je zajimave jak se CR posouva do SR(Spec review).
Uplne krasny je, kdyz dokazu dodat tu specku ktera ma v sobe u kazdeho usecase tlacitko ktere spusti odpovidajici test case. Na to si uzivaci hrozne rychle zvykaji a pri dostavam krasnej feedback a kutej mi tam corner cases kde to zkousej rozbit..
-
"Ex post" code review absolutně nefunguje, protože pak už nikdo nemá žádný důvod se mu věnovat. Jediné co funguje je vydírání tím že nebude povolený merge dokud se nedořeší code review.
-
Ale funguje... jen zalezi na tymove kulture.
-
Delate code review pred merge nebo az po nem?
1. Vede to na to, ze lidem se nechce a zacnou delat formalni approvaly.. nebo approvujou podle toho od koho je MR...
...
Uáááá! Pokud to mergneš před review, tak všechny ty body, co vypisuješ, jsou úplně nic proti "však to už je na produkci a funguje, tak proč bych na tom review trávil půl dne." Review je buď požadavek na merge, nebo nemá smysl.
A pak to "ex post" code review muze naopak mit velky prinos.. udela se jednou za cas meeting kde se posedi a dohromady se probere co jde spravnym smerem a co jsou veci ktery by se radsi meli opravit... a vysvetli se proc a treba se to muze zdokumentovat pokud jsou v guidelines nejaky diry...
To není code review, to je možná tak architecture review, long term planning, nebo něco takového. Code review má chytit problémy (a problém není vždy jen bug zachytitelný testem) PŘED tím, než se ten kód oživí na produkčním prostředí. Ostatně, jak bez code review víš, že ta nová funkcionalita vůbec má nějaké testy a ty testy reálně něco dělají? Pak už je na to pozdě. Prevence lepší lékaře a tak.
-
Jeste je zajimave se na to podivat z jineho pohledu.
Delate code review pred merge nebo az po nem?
Když jsem kdysi pracoval ve startupu, tak tam nebylo vůbec žádný code review a commitovalo se rovnou do masteru. Jak se firma rozrůstala, tak se přidalo code review, ale commitovalo se pořád rovnou do masteru. Výsledek: junioři se toho báli a stejně si věci dělali ve větvi a nechávali schválit před mergem, master byl věčně rozbitej (padaly testy nebo vůbec nešel zkompilovat), často nešlo nasadit do produkce, protože to nepustily sanity checky (který jsme naštěstí měli celkem robustní). Pro mě jako seniornějšího člověka to ale bylo pohodlný, protože jsem nemusel řešit problémy typu "dělám na věci X, ale potřebuju použít něco, co jsem udělal na věci Y, která ještě není zamergovaná".
Jinak k původní otázce: Jdu commit po commitu a čtu poctivě všechno. Kontroluju hlavně čitelnost a best practices. Jestli kód dělá to, co má, ověřujou testy (který ale taky kontroluju důkladně).
-
Jak se firma rozrůstala, tak se přidalo code review, ale commitovalo se pořád rovnou do masteru. Výsledek: junioři se toho báli a stejně si věci dělali ve větvi a nechávali schválit před mergem, master byl věčně rozbitej (padaly testy nebo vůbec nešel zkompilovat)
Jinak řečeno, sice byly testy, ale lokálně neužitečné, protože sis je stejně nemohl pustit, dokud někdo jiný neopravil svou kupku hnoje. :D
My máme mergnutí do masteru/release větví podmíněné nejen review, ale i zelenou CI a approval od statické analýzy, která mimo jiné například hlídá zakázané patterny, použití ne-cryptograficky-bezpečných funkcí , nebo detekuje nové použití označených deprecated tříd a v tu chvíli vyžaduje na review approval od teamu, co vlastní tu deprecated třídu. Výsledek je ten, že k rozbití masteru prakticky nedochází (vždycky se můžou sejít dvě navzájem nekompatibilní změny dřív, než vyprší životnost CI buildu, ale to je výjimka). Ale taky je fakt, že nejsme startup a máme týmy lidí na věci, které ve startupu dělá jeden člověk, když mu zbyde prostor v pátek odpoledne.
-
Řekl bych, že aplikuji několik "strategií" code-review na základě pár informací, které zjistím rychle
- Kdo je autorem - na základě toho už přibližně vím, co mě čeká a jakou kvalitu můžu očekávat.
- Čeho se změny týkají - zasahuje to do mého modulu, do společných věcí, do modulu jiného týmu, ...
- Testy - má to vůbec nějaké? Jsou test-casy srozumitelné? Je zvolen správný typ testu? ...
- git historie - když tam např vidím 10 commitů s message "fix" pár minut po sobě, jsem ostražitější
Tyhle věci co mi zaberou maximálně pár minut rozhodnou o tom jak zevrubně dělám review. U kódu který se týká mého modulu a společných věcí buď schvaluji nebo žádám změny, u cizího modulu - pokud to vyloženě není odpad - změny nežádám (blokuje merge) a nechávám rozhodnout jiné vývojáře
-
3. Vede to na to, ze se tolik netestuje a automatizace je na druhe koleji...
Tenhle bod hlavně, ale i celý příspěvek je psaný z pohledu jako kdyby code reviews byly nějaká náhrada testů. To je za mě naprosto chybný pohled, code reviews mohou nějaký bug sem tam zachytit, ale primárně jsou zaměřené na úplně jiné věci. Na to, jak smysluplné automatické testy jsou, jak moc pochopitelný je kód, jestli jsou proměnné a funkce dobře pojmenované, high-level věci, různé pasti... My třeba máme kód, co běží v mnoha různých deploymentech a chová se na nich různě, což se přepíná feature flagy. Nedávno jsem jednomu juniorovi vrátil kód, kde testoval, zda je na konkrétním deploymentu podle jeho jména místo co by zavedl feature flag. Testy by procházely, ale code review tohle nesmí projít.
4. hrozne to zpomaluje flow... (mam treba rozdelany 3 pull requesty zaroven a tlacim je prez code review 3 dny nez si lidi udelaji cas na review zapracuju pozadovany zmeny apod.. )
To záleží na týmové kultuře a je to práce team leada, aby se pull requesty na code review zbytečně nesekaly, stejně jako aby si v nich programátoři nepoměřovali pindíky a nehádali se o blbostech.
Mimochodem, AI je na code review celkem užitečná - konkrétně hezky zachytí například když člověk zkopíruje kód, ale zapomene upravit komentář. Ale je dobré jako doplněk - občas reportuje nesmysly a samozřejmě dost z věcí co jsem psal výš že code review má kontrolovat neodchytí.
-
Delate code review pred merge nebo az po nem?
1. Vede to na to, ze lidem se nechce a zacnou delat formalni approvaly.. nebo approvujou podle toho od koho je MR...
...
Uáááá! Pokud to mergneš před review, tak všechny ty body, co vypisuješ, jsou úplně nic proti "však to už je na produkci a funguje, tak proč bych na tom review trávil půl dne." Review je buď požadavek na merge, nebo nemá smysl.
A pak to "ex post" code review muze naopak mit velky prinos.. udela se jednou za cas meeting kde se posedi a dohromady se probere co jde spravnym smerem a co jsou veci ktery by se radsi meli opravit... a vysvetli se proc a treba se to muze zdokumentovat pokud jsou v guidelines nejaky diry...
To není code review, to je možná tak architecture review, long term planning, nebo něco takového. Code review má chytit problémy (a problém není vždy jen bug zachytitelný testem) PŘED tím, než se ten kód oživí na produkčním prostředí. Ostatně, jak bez code review víš, že ta nová funkcionalita vůbec má nějaké testy a ty testy reálně něco dělají? Pak už je na to pozdě. Prevence lepší lékaře a tak.
Doporucuju nastudovat neco o continuous delivery.
Muzes mit automatizovane testy, ktere ti otestuji jestli mas testy a jestli ty testy testuji co maji testovat viz napr. mutation a coverage testing.
To co popisujes ty znaci, ze nemas dostatecnou duveru v pipelinu.
Pokud je to na produkci a funguje to tak je to v poradku a muze to tam byt.. .proc bych travil cas na code review kterym budu blokovat merge?
Pokud je to hnusno-kod kterej tam nechci i kdyz funguje tak ho proste oznacim jak technical friction ktery je potreba resit a udelam ta to ticket do pristiho sprintu... S nikym nemusim diskutovat jestli se to udela nebo ne... projektova kultura proste rika, ze tohle se bude resit... A navic muzu pridat automatizovanej test na to aby uz se to priste nestalo...
Casem se muze stat ze ta pipelina sama bezi moc dlouho tak se musi resit ktere veci se budou kontrolovat s kazdym MR a ktere veci pobezi prez noc...
Ale jak sem uz psal... plati to jen pro veci ktere muzu takhle delat a pokud mam projektovou kulturu pripravenou na takovy proces...
#MoveFastAndBreakThings
-
Muzes mit automatizovane testy, ktere ti otestuji jestli mas testy a jestli ty testy testuji co maji testovat viz napr. mutation a coverage testing.
To co popisujes ty znaci, ze nemas dostatecnou duveru v pipelinu.
Pokud je to na produkci a funguje to tak je to v poradku a muze to tam byt.. .proc bych travil cas na code review kterym budu blokovat merge?
Review ale nemá primárně chytat nějaké testovatelné bugy, jak už tu zaznělo několikrát. Review potřebuješ dělat i u 100% covered codebase, protože jen neuronka (ať už biologická, nebo umělá) může posoudit, jestli jsou věci smysluplně pojmenované, rozdělené, jestli je to udržitelně napsané, jestli ty testy jsou napsané tak, aby reálně detekovaly chyby a přitom nezávislé změny nevyžadovaly hromadu oprav testů...
Jasně, mutation a fuzzy testing je v principu super, ale pokud i bez nich plný test suite běží hodiny a hodiny, tak plošná mutabilita to násobí, takže se to dá používat jen cíleně.
Pokud je to hnusno-kod kterej tam nechci i kdyz funguje tak ho proste oznacim jak technical friction ktery je potreba resit a udelam ta to ticket do pristiho sprintu... S nikym nemusim diskutovat jestli se to udela nebo ne... projektova kultura proste rika, ze tohle se bude resit... A navic muzu pridat automatizovanej test na to aby uz se to priste nestalo...
Pokud není čas na "udělat to pěkně už na prvním MR", tak stoprocentně nebude čas na přepsání později, kdy to začnou používat další věci. Tyhle tickety se přištích pár měsíců přesouvají ze sprintu do sprintu, pak se to přesune na plán na příští rok a pak se zavřou jako "not relevant after 5 years".
Casem se muze stat ze ta pipelina sama bezi moc dlouho tak se musi resit ktere veci se budou kontrolovat s kazdym MR a ktere veci pobezi prez noc...
Ano. To má být jako nějak špatně, nebo co?
#MoveFastAndBreakThings
Jo, při rozhlédnutí se je vidět, kolik lidí a firem za tuhle myšlenku schovává nekvalitní práci a věčně akumulující se technický dluh.
-
Pokud není čas na "udělat to pěkně už na prvním MR", tak stoprocentně nebude čas na přepsání později, kdy to začnou používat další věci. Tyhle tickety se přištích pár měsíců přesouvají ze sprintu do sprintu, pak se to přesune na plán na příští rok a pak se zavřou jako "not relevant after 5 years".
Tohle je ten problem. Dokud to mate takhle tak mas pravdu a musite delat gatekeeping reviews.
Co to znamena "se presouvaji?" Tohle proste neni k diskuzi. Kdyz to nekdo zkusi proste to presunu zpatky a vyrikam si s nima, at mi do toho nesahaji.
-
Pokud není čas na "udělat to pěkně už na prvním MR", tak stoprocentně nebude čas na přepsání později, kdy to začnou používat další věci. Tyhle tickety se přištích pár měsíců přesouvají ze sprintu do sprintu, pak se to přesune na plán na příští rok a pak se zavřou jako "not relevant after 5 years".
Tohle je ten problem. Dokud to mate takhle tak mas pravdu a musite delat gatekeeping reviews.
Co to znamena "se presouvaji?" Tohle proste neni k diskuzi. Kdyz to nekdo zkusi proste to presunu zpatky a vyrikam si s nima, at mi do toho nesahaji.
Máš tedy možnost pozastavit vývoj nových fíčur, aby vám nerostl maintenance cost?
-
Pokud není čas na "udělat to pěkně už na prvním MR", tak stoprocentně nebude čas na přepsání později, kdy to začnou používat další věci. Tyhle tickety se přištích pár měsíců přesouvají ze sprintu do sprintu, pak se to přesune na plán na příští rok a pak se zavřou jako "not relevant after 5 years".
Tohle je ten problem. Dokud to mate takhle tak mas pravdu a musite delat gatekeeping reviews.
Co to znamena "se presouvaji?" Tohle proste neni k diskuzi. Kdyz to nekdo zkusi proste to presunu zpatky a vyrikam si s nima, at mi do toho nesahaji.
Máš tedy možnost pozastavit vývoj nových fíčur, aby vám nerostl maintenance cost?
A Ty ne?
-
Pokud není čas na "udělat to pěkně už na prvním MR", tak stoprocentně nebude čas na přepsání později, kdy to začnou používat další věci. Tyhle tickety se přištích pár měsíců přesouvají ze sprintu do sprintu, pak se to přesune na plán na příští rok a pak se zavřou jako "not relevant after 5 years".
Tohle je ten problem. Dokud to mate takhle tak mas pravdu a musite delat gatekeeping reviews.
Co to znamena "se presouvaji?" Tohle proste neni k diskuzi. Kdyz to nekdo zkusi proste to presunu zpatky a vyrikam si s nima, at mi do toho nesahaji.
Máš tedy možnost pozastavit vývoj nových fíčur, aby vám nerostl maintenance cost?
A Ty ne?
sním o tom, ale tech lead má malou vyjednávací pozici :/
-
Pokud není čas na "udělat to pěkně už na prvním MR", tak stoprocentně nebude čas na přepsání později, kdy to začnou používat další věci. Tyhle tickety se přištích pár měsíců přesouvají ze sprintu do sprintu, pak se to přesune na plán na příští rok a pak se zavřou jako "not relevant after 5 years".
Tohle je ten problem. Dokud to mate takhle tak mas pravdu a musite delat gatekeeping reviews.
Co to znamena "se presouvaji?" Tohle proste neni k diskuzi. Kdyz to nekdo zkusi proste to presunu zpatky a vyrikam si s nima, at mi do toho nesahaji.
Máš tedy možnost pozastavit vývoj nových fíčur, aby vám nerostl maintenance cost?
Jo a ne, ale ani to nepotrebuju. Vetsinou to funguje bez problemu, ale eskalacni cesta by asi byla takova, ze tech lead prijde za mnou az po tom co ho product manager tlaci ze ma deprioritizovat neco na ukor ficur. Ja si pak promluvim s product managerem a ukazu mu kde mame zdokumentovanej proces a kde jsem se vsichni shodli jak to budeme delat a ze ted nemuzem zacit "sekat rohy" a kdyz nahodou by na tom takovej PM trval tak pujdu za cluster leadem a udelam na to risk ticket, ale takhle daleko to nikdo nenecha zajit...
-
Tenhle bod hlavně, ale i celý příspěvek je psaný z pohledu jako kdyby code reviews byly nějaká náhrada testů. To je za mě naprosto chybný pohled, code reviews mohou nějaký bug sem tam zachytit, ale primárně jsou zaměřené na úplně jiné věci. Na to, jak smysluplné automatické testy jsou, jak moc pochopitelný je kód, jestli jsou proměnné a funkce dobře pojmenované, high-level věci, různé pasti... My třeba máme kód, co běží v mnoha různých deploymentech a chová se na nich různě, což se přepíná feature flagy. Nedávno jsem jednomu juniorovi vrátil kód, kde testoval, zda je na konkrétním deploymentu podle jeho jména místo co by zavedl feature flag. Testy by procházely, ale code review tohle nesmí projít.
Urcite kod review neni nahrada za testy. (Takze tohle je tak trochu strawman.. ale)
Spoustu se toho ale da otestovat treba ArchUnitem...
Ale k tvemu konkretnimu prikladu....
Fungovalo by to na produkci? To ze se to testuje podle jmena a neni tam feature flag? Nebo by to zpusobilo outage?
Pokud by to fungovalo tak to tam chvilku muze vydrzet a vyresim to v ex-post code review a opravim v pristim sprintu.
Pokud by to nefungovalo zachyti to treba uz ArgoCD rollouts nebo neco takoveho pri nasazeni na 10% a provede se rollback.
A ano.. uznavam, ze se muze stat ze proste neco utece a zpusobi to skody v produkci, ale...
1. nejednou se mi to stalo i kdyz jsme delali gatekeeping code reviews takze to urcine neni vsespasne...
2. Tim, ze s tim pocitam, si zase dam zalezet na monitoringu a alertingu... a jakmile se objevi anomalie tam muzu zareagovat a rychle to opravit nebo odrolovat...
Taky je dobry treba mit v pipeline alerty na to, ze neco potrebuje approval.. treba migracni scripty je asi lepsi kontrolovat protoze se to dost casto blbe odrolovava.
-
Nastastie aktualne na projekte pracujeme 2ja, kazdy na trochu inej urovni, ale tiez fungujeme na CR, pretoze stane sa, ze v ramci skusania zabudem odstranit nejaky zbytocny kod, alebo tam ostane nejaka "debug hlaska".
V principe, ale vzdy sa nejak vieme dohodnut a drzime si nejaky standard, co sa tyka kodu. Pri vacsich zmenach datam komentare, co som tym vlastne chcel docielit a hlavne, prerabky robim postupne, tak davam aj komenty typu: viem, toto nie je ideal, ale su tam este dalsie zavislosti, ktore sa riesia v inych ulohach a toto je medzikrok.
V predchadzajucej firme mali CI/CD s nejakymi nastrojmi na kontrolu a pri commite sa testovalo aj to, ci je mozne urobit build. takze ak niekto chcel poslat nefunkcny kod, commit ho rovno odmietol. Sice trvalo to par sekund dlhsie, ale aspon bolo iste, ze sa ten program spusti.
-
Máme code-review jako součást pull-requestu a chápeme to jako formu komunikace mezi vývojáři. Ve smyslu "ten úkol, co jsme minule diskutovali, jsem vyřešil takto, je to za Tebe OK?" nebo "tohle bych potřeboval dostat do upstream, co si o tom myslíš?". Merge u nás nejde potvrdit, pokud neprošli testy a různé kontroly kvality (součást pipeliny), release candidate se pak delší dobu testuje v neprodukčním prostředí a v případě potřeby iterativně opravuje.
-
Code review? https://tenor.com/view/mrknu-a-vidim-mrknu-a-vid%C3%ADm-s%C4%8Derty-nejsou%C5%BEerty-cert-%C4%8Dert-gif-21579181
-
Ale k tvemu kohnkretnimu prikladu....
Fungovalo by to na produkci? To ze se to testuje podle jmena a neni tam feature flag? Nebo by to zpusobilo outage?
Pokud by to fungovalo tak to tam chvilku muze vydrzet a vyresim to v ex-post code review a opravim v pristim sprintu.
Fungovalo by to, ale proč dávat na produkci nekvalitní kód a pak se k němu zase vracet a neudělat to pořádně napoprvé? Co se stane, když se to na produkci dostane o den později? Pokud to není hotfix na kritický problém tak nic a když se to opraví hned, tak je to (vesměs) méně pracné, než to znovu otvírat po týdnu / dvou, kdy člověk už většinou nemá kontext v hlavě.
-
Dovolim si delegovat odpoved na na Davea... beztak to rekne lip nez ja i kdyz s nim uplne ve vsem nesouhlasim:
https://www.youtube.com/watch?v=WmVe1QrWxYU (https://www.youtube.com/watch?v=WmVe1QrWxYU)
-
Dovolim si delegovat odpoved na na Davea... beztak to rekne lip nez ja i kdyz s nim uplne ve vsem nesouhlasim:
https://www.youtube.com/watch?v=WmVe1QrWxYU (https://www.youtube.com/watch?v=WmVe1QrWxYU)
A mohl bys prosím napsat, co vidíš jinak než on?
-
Dovolim si delegovat odpoved na na Davea... beztak to rekne lip nez ja i kdyz s nim uplne ve vsem nesouhlasim:
https://www.youtube.com/watch?v=WmVe1QrWxYU (https://www.youtube.com/watch?v=WmVe1QrWxYU)
V první minutě řekl takovou spoustu nesmyslů, že jsem to nevydržel a vypnul, sorry.
-
Dovolim si delegovat odpoved na na Davea... beztak to rekne lip nez ja i kdyz s nim uplne ve vsem nesouhlasim:
https://www.youtube.com/watch?v=WmVe1QrWxYU (https://www.youtube.com/watch?v=WmVe1QrWxYU)
A mohl bys prosím napsat, co vidíš jinak než on?
Ohledne CD si ani netroufam s nim nesouhlasit... spis jsem jeste nedorostl do takove urovne abych to uplne vsechno docenil, ale spis se rozchazime v pohledu na AI.. prijde mi prilis konzervativni.
-
Dovolim si delegovat odpoved na na Davea... beztak to rekne lip nez ja i kdyz s nim uplne ve vsem nesouhlasim:
https://www.youtube.com/watch?v=WmVe1QrWxYU (https://www.youtube.com/watch?v=WmVe1QrWxYU)
V první minutě řekl takovou spoustu nesmyslů, že jsem to nevydržel a vypnul, sorry.
njn... s tim pak ja nic nenadelam.
-
Dovolim si delegovat odpoved na na Davea... beztak to rekne lip nez ja i kdyz s nim uplne ve vsem nesouhlasim:
https://www.youtube.com/watch?v=WmVe1QrWxYU (https://www.youtube.com/watch?v=WmVe1QrWxYU)
Tak to jsem teda hodne nepochopil. To jako ze vyvinu nejakou featuru, zamerguju a pak dodatecne budu delat code review?
- minimalne se prodrazi manualni testovani. Nebo uz neni potreba pak retestovat, pokud do te featury znova sahnu?
- dodatecnym fixem muzu zanest bug (100% code coverage jeste neznamena 100% spravnost, zvlast v dobe AI)
-
- minimalne se prodrazi manualni testovani. Nebo uz neni potreba pak retestovat, pokud do te featury znova sahnu?
- dodatecnym fixem muzu zanest bug (100% code coverage jeste neznamena 100% spravnost, zvlast v dobe AI)
listoper to tady už vysvětloval. Princip je v tom, že ti testery dělají uživatelé. Prostě spoléháš jen na automatické testy a pokud něco přesto rozbiješ, tak to uživatelé nahlásí a ty to opravíš. Když si vzpomenu, jakým způsobem v poslední době funguje Microsoft, Facebook a další, tak mi přijde, že tohle workflow se opravdu používá.
-
... tak to uživatelé nahlásí a ty to opravíš...
Vtip je predevsim v tom, ze ty nic neopravis, maximalne userum ukazes vztyceny prostrednik. A presne takhle to dela prave (a zdaleka nejen) MS.
Zrovna nedavno reportoval kolegacek bug dodavateli, na kterym bylo zjevny, ze to nikdo ani jednou ani nezkusil spustit. Meli tam syntax error v sql na kterym to ihned po spusteni zbuchlo. Pricemz se bavime o tom, ze si tohle umim odchytit a tudiz to dodavateli radne omlatit o hlavu. To beznej user neumi, tomu to proste jen pada.
-
Dovolim si delegovat odpoved na na Davea... beztak to rekne lip nez ja i kdyz s nim uplne ve vsem nesouhlasim:
https://www.youtube.com/watch?v=WmVe1QrWxYU (https://www.youtube.com/watch?v=WmVe1QrWxYU)
Tak to jsem teda hodne nepochopil. To jako ze vyvinu nejakou featuru, zamerguju a pak dodatecne budu delat code review?
- minimalne se prodrazi manualni testovani. Nebo uz neni potreba pak retestovat, pokud do te featury znova sahnu?
- dodatecnym fixem muzu zanest bug (100% code coverage jeste neznamena 100% spravnost, zvlast v dobe AI)
Ja to zkusim zjednodusit, i kdyz predpokladam ze to bude moc a nekam to utece, ale treba to vysvetli tenhle dotaz...
Zkus si to predstavit tak, ze jen zmenim poradi kroku (viz priloha)
Proste to zamerguju schovany za feature flag.
Objem prace zustane stejnej jen se zmeni poradi...
-
- minimalne se prodrazi manualni testovani. Nebo uz neni potreba pak retestovat, pokud do te featury znova sahnu?
- dodatecnym fixem muzu zanest bug (100% code coverage jeste neznamena 100% spravnost, zvlast v dobe AI)
Princip je v tom, že ti testery dělají uživatelé. Prostě spoléháš jen na automatické testy a pokud něco přesto rozbiješ, tak to uživatelé nahlásí a ty to opravíš.
Skoro... ten manual testing tam porad muzes mit (viz ten muj predchozi prispevek).
Ale urcite se daleko vic spoleha na automation.
-
... tak to uživatelé nahlásí a ty to opravíš...
Vtip je predevsim v tom, ze ty nic neopravis, maximalne userum ukazes vztyceny prostrednik. A presne takhle to dela prave (a zdaleka nejen) MS.
Zrovna nedavno reportoval kolegacek bug dodavateli, na kterym bylo zjevny, ze to nikdo ani jednou ani nezkusil spustit. Meli tam syntax error v sql na kterym to ihned po spusteni zbuchlo. Pricemz se bavime o tom, ze si tohle umim odchytit a tudiz to dodavateli radne omlatit o hlavu. To beznej user neumi, tomu to proste jen pada.
Zrovna syntax error by se mel dat chytit i jinak nez ze to nekdo bude poustet...
Takze bych si vsadil, ze za tim nebude continuous delivery.
A skoro bych si vsadil na to ze to i v pohode proslo prez code review ;D
-
Dovolim si delegovat odpoved na na Davea... beztak to rekne lip nez ja i kdyz s nim uplne ve vsem nesouhlasim:
https://www.youtube.com/watch?v=WmVe1QrWxYU (https://www.youtube.com/watch?v=WmVe1QrWxYU)
Tak to jsem teda hodne nepochopil. To jako ze vyvinu nejakou featuru, zamerguju a pak dodatecne budu delat code review?
- minimalne se prodrazi manualni testovani. Nebo uz neni potreba pak retestovat, pokud do te featury znova sahnu?
- dodatecnym fixem muzu zanest bug (100% code coverage jeste neznamena 100% spravnost, zvlast v dobe AI)
Ja to zkusim zjednodusit, i kdyz predpokladam ze to bude moc a nekam to utece, ale treba to vysvetli tenhle dotaz...
Zkus si to predstavit tak, ze jen zmenim poradi kroku (viz priloha)
Proste to zamerguju schovany za feature flag.
Objem prace zustane stejnej jen se zmeni poradi...
Takze vlastne devel/main branch je stale rozbita?
-
Dovolim si delegovat odpoved na na Davea... beztak to rekne lip nez ja i kdyz s nim uplne ve vsem nesouhlasim:
https://www.youtube.com/watch?v=WmVe1QrWxYU (https://www.youtube.com/watch?v=WmVe1QrWxYU)
Tak to jsem teda hodne nepochopil. To jako ze vyvinu nejakou featuru, zamerguju a pak dodatecne budu delat code review?
- minimalne se prodrazi manualni testovani. Nebo uz neni potreba pak retestovat, pokud do te featury znova sahnu?
- dodatecnym fixem muzu zanest bug (100% code coverage jeste neznamena 100% spravnost, zvlast v dobe AI)
Ja to zkusim zjednodusit, i kdyz predpokladam ze to bude moc a nekam to utece, ale treba to vysvetli tenhle dotaz...
Zkus si to predstavit tak, ze jen zmenim poradi kroku (viz priloha)
Proste to zamerguju schovany za feature flag.
Objem prace zustane stejnej jen se zmeni poradi...
No asi bych nemel klidne spani pokud bych vyvijel treba processing platebnich transakci a po zamergovani pak delal jeste nejake fixupy kvuli code review. Uprimne, podle me by to ani nikdo pricetny neschvalil takovy development proces.
Pro nejake min kriticke aplikace asi ok. Koneckoncu driv se ani code review nedelalo a ani automatizovane testy.
-
Takze vlastne devel/main branch je stale rozbita?
Naopak... je porad releasable.
-
Proste to zamerguju schovany za feature flag.
To ale trochu předpokládá, že všechny úpravy, co jsem dělal, jdou schovat za feature flag. Z mojí zkušenosti přidání skoro každé feature předchází refactoring, aby moc nevyčnívala. A ten taky může rozbít věci, a to věci, který už jsou "otestovaný". Takže bych čekal, že tenhle workflow povede víc ke kultuře "na fungující věci se nesahá" a kód bude rochleji nabírat dluh.
Samozřejmě záleží hodně na složení týmu a někde, kde jsou samí seniornější lidi, bych si to asi dokázal představit, pokud by se fakt hodně práce věnovalo automatickým testům (ale zase - ty taky je potřeba refaktorovat, a co když rozbiju test, a ten pak nechytí problém?)
-
Ja musim znova doporucit kouknout na nejaky videa od toho Dava Farley.. a taky si treba projit reference co uvadi v description.
On to nema vycucany z prstu.
Dost casto tam zminuje a odkazuje ruzny publikace a papery a tam jsou statisticky podlozeny "dukazy" ze to funguje.
Samozrejme ze zalezi na kulture firmy, seniorite teamu atd atd.
To ze neco zni "z moji zkusenosti" divne, nebo i strasidelne neznamena nutne ze to tak je.
A kdyz postavim zkusenost jednotlivce proti vedecke praci kolektivu postavene na vzorku stovek nebo i tisicu lidi, tak "z moji zkusenosti" se vetsinou vyplati verit ty studii i kdyz ten jednotlivec bude moje moudra babicka.
A pokud treba Dave Farley neni na prvni poslech stravitelnej tak muzu jeste doporucit Kevlin Henney nebo Dan North.
Jsou o dost zabavnejsi a taky se tech temat obcas dotknou, ale mene invazivni formou...
-
Takze vlastne devel/main branch je stale rozbita?
Naopak... je porad releasable.
On je tam nezanedbatelný překryv. A záleží primárně na podmínkách ve smlouvách, míře monopolního postavení a dalších nesoftwarových věcech.
Standardní euly pro plebs se přece píšou tak, aby bylo releasnutelné úplně cokoliv.
-
Takze vlastne devel/main branch je stale rozbita?
Naopak... je porad releasable.
To jo, ale ptal jsem se na hlavni vetev. Tam po mergi budu ji budu mit ihned rozbitou ne?
-
Takze vlastne devel/main branch je stale rozbita?
Naopak... je porad releasable.
To jo, ale ptal jsem se na hlavni vetev. Tam po mergi budu ji budu mit ihned rozbitou ne?
Ja taky mluvim o hlavni vetvi... idealne jedinne vetvi... Proc by mela byt rozbita?
-
Takze vlastne devel/main branch je stale rozbita?
Naopak... je porad releasable.
To jo, ale ptal jsem se na hlavni vetev. Tam po mergi budu ji budu mit ihned rozbitou ne?
Ja taky mluvim o hlavni vetvi... idealne jedinne vetvi... Proc by mela byt rozbita?
No podle toho obrázku se nějaký review řeší až po merge. Ale tuším, že mi uniká nějaká hlavní myšlenka tohoto workflow.
https://forum.root.cz/index.php?action=dlattach;topic=31593.0;attach=8122;image
-
No podle toho obrázku se nějaký review řeší až po merge. Ale tuším, že mi uniká nějaká hlavní myšlenka tohoto workflow.
https://forum.root.cz/index.php?action=dlattach;topic=31593.0;attach=8122;image
Ja to vedel, ze kdyz to zkusim zjednodusit tak to nedopadne.
Zkusim doplnit v poznamkach:
1. Pair/Mob programming > code review => tzn idealne probiha "code review" soubezne s developmentem
2. Pokud to projde vsema automatizovanejma testama tak to neni rozbity a muze to jit na produkci, ataky to tam jde... treba 10x denne (mozna schovany za feature flagem)
3. Pokud je to "rozbity" i kdyz to proslo pipelinou tak musim vylepsit pipelinu ;-)
Asi zase doplnim odkaz https://www.youtube.com/watch?v=CR3LP2n2dWw (https://www.youtube.com/watch?v=CR3LP2n2dWw)
-
No podle toho obrázku se nějaký review řeší až po merge. Ale tuším, že mi uniká nějaká hlavní myšlenka tohoto workflow.
https://forum.root.cz/index.php?action=dlattach;topic=31593.0;attach=8122;image
Ja to vedel, ze kdyz to zkusim zjednodusit tak to nedopadne.
Zkusim doplnit v poznamkach:
1. Pair/Mob programming > code review => tzn idealne probiha "code review" soubezne s developmentem
2. Pokud to projde vsema automatizovanejma testama tak to neni rozbity a muze to jit na produkci, ataky to tam jde... treba 10x denne (mozna schovany za feature flagem)
3. Pokud je to "rozbity" i kdyz to proslo pipelinou tak musim vylepsit pipelinu ;-)
Asi zase doplnim odkaz https://www.youtube.com/watch?v=CR3LP2n2dWw (https://www.youtube.com/watch?v=CR3LP2n2dWw)
ok to už začíná dávat smysl, dokážu si i představit, jak by mělo vypadat složení takového týmu. Dík
-
Objem prace zustane stejnej jen se zmeni poradi...
Tohle je jen teorie. Code review po mergi je asi hezky predpoklad, ale nebude ve vetsi mire fungovat.
-
Objem prace zustane stejnej jen se zmeni poradi...
Tohle je jen teorie.
Vsak jsem psal ze je to zjednodusene a jen na vysvetleni toho jednoho pointu...
Code review po mergi je asi hezky predpoklad, ale nebude ve vetsi mire fungovat.
Dave Farley rika, ze jo a doklada to nekolika case studies kde uz jim to zafungovalo a tim padem ja uz nemusim nic dokazovet :P
-
Dave Farley rika, ze jo a doklada to nekolika case studies kde uz jim to zafungovalo a tim padem ja uz nemusim nic dokazovet :P
Tak jeho prace je mit videa generujici penize, takze bych ho na 100% nebral vazne. Proste udela viralni video a o par mesicu pozdeji to otoci (kdysi takto otacel ohledne mikrosluzeb).
-
... a doklada to nekolika case studies ...
-
Dave Farley rika, ze jo a doklada to nekolika case studies kde uz jim to zafungovalo a tim padem ja uz nemusim nic dokazovet :P
Nekolik case studies != ve vetsi mire.
Stejne jako existence nekolika tyrkysovych nerika nic o tom, ze by to melo fungovat ve vetsi mire.
-
Dave Farley rika, ze jo a doklada to nekolika case studies kde uz jim to zafungovalo a tim padem ja uz nemusim nic dokazovet :P
Nekolik case studies != ve vetsi mire.
Stejne jako existence nekolika tyrkysovych nerika nic o tom, ze by to melo fungovat ve vetsi mire.
Hele ja uz toho asi necham, protoze me to neba, ale musim se jeste zeptat.
Existuje neco co ja ted muzu napsat nebo rict abys tomu 'otevrel mysl'?
Protoze ja tady posilam odkazy atp a ty jen vagne reknes "to nebude ve vetsi mire fungovat"....
S tim se neda moc pracovat...
-
Dekuji, o tuto investici nemam zajem.