chilek / lms

Lan Management System (LMS) public GIT repo
http://lms.org.pl
126 stars 135 forks source link

remove legacy code using automatic code refactoring quality tools (third bunch) #2465

Closed interduo closed 7 months ago

chilek commented 7 months ago

Nie - nie będę robił review po AI. Znalazłem w Twoich commitach przesłąnych bezpośrednio do repo już na początku kilka błędów merytorycznych i mi się odechciało dalej sprawdzać. AI nie zastąpi wiedzy i doświadczenia programisty, gdyż na obecnym etapie nie jest w stanie zrozumieć kontekstu - gubi się na tak elementarnych rzeczach jak isset(). Kluczem jest fakt, że isset() może mieć na celu nie tylko sprawdzenie czy zmienna istnieje, ale również czy ma wartość inną od null.

interduo commented 7 months ago

To nie jest AI a po prostu reguły właśnie napisane przez programistów z doświadczeniem.

Celowo podzieliłem zmiany per reguła a nie władowałem wszystko w jeden worek (używając grup reguł), żeby łatwo było wychwycić ewentualny problem i poprawić regułę/omówić.

Kluczem jest fakt, że isset() może mieć na celu nie tylko sprawdzenie czy zmienna istnieje, ale również czy ma wartość inną od null.

Konkrety, pokaż zmianę/commit, która generuje problem. Z chęcią opiszę issue z wrzutką minimalnego kodu powodujące go problem na repo rectora.

Może mówisz o którymś z tych przypadków: a) isset($zmienna) w pliku w którym kilka linii wyżej jest $zmienna = array(); b) w podwójnym warunku łączonym spójnikiem OR z próbą !empty()? ?

Commity nie były przesłane bezpośrednio do repo ale kliknąłem PR po kilku dniach, żeby się nie zakonfliksiły.

Patrz jest PHP8.x na którym nie działa cześć funkcji LMSa. Próbowałem używać go kilka dni latając lokalnie tu i tam kod. Szkoda czasu na taką formę pracy. Trzeba automatyzować...

Rzuć okiem na ten PR https://github.com/chilek/lms/pull/2464 swoim doswiadczeniem zmiany są rodzajowe/generatywne. Omówione konkretne reguły tutaj: https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md Cenie Twoje doświadczenie ale trzeba kroczyć dalej i uzupełniać je nowościami.

Zrób git cherry pick tych, które uznasz za dobre. Takie narzędzia to krok milowy w programowaniu.... Dodam, że cześć tych reguł/testów jest również dostępna w phpcbf, którego używasz.

Serio nie jestem w stanie uwierzyć że programiści piszący te reguły się mylą w każdej z nich ;-) To jest mega narzędzie, korzysta z niego drupal, sympfony i inni duzi.

interduo commented 7 months ago

Popuszczam każdy z tych commitów oddzielnym PR. Omówimy każdą rodzajową zmianę i potencjalne problemy.