KNR-PW / discord-bot

KNR discord bot
MIT License
0 stars 1 forks source link

Spakowanie programu w mniejsze moduły. #20

Closed PiotrWeppo closed 1 year ago

PiotrWeppo commented 1 year ago

Jak w tytule. Powinno polepszyć czytelność programu.

https://github.com/KNR-PW/discord-bot/issues/8

PiotrWeppo commented 1 year ago

Hmm, mam zagwozdkę. Cały czas embed_creator i embed_methods mają błąd "circular imports". Gdybym je spakował razem to nadal moduł będzie miał poniżej 1000 wierszy (ok 750). Bardziej wolałbym przebudować kod, któregoś, ale nie wiem jak to ugryźć przy takim zaawansowaniu.

PiotrWeppo commented 1 year ago

Próbowałem zrobić rebase przy pomocy gitgrapha, ale chyba nic z tego.

pktiuk commented 1 year ago

Próbowałem zrobić rebase przy pomocy gitgrapha, ale chyba nic z tego

Dziwne, u mnie działa image

pktiuk commented 1 year ago

Czemu robisz pull requesta ze swojego brancha roboczego (na własnym repo) na innego brancha roboczego (na tym repo)?

PiotrWeppo commented 1 year ago

Próbowałem zrobić rebase przy pomocy gitgrapha, ale chyba nic z tego

Dziwne, u mnie działa image

Chciałem po prostu zmienić tamtego commita bez dodawania nowego. Wydaje mi się, że użyłem tej opcji i nic nie zmieniła na stronie githuba, więc zrobiłem później push branch, które z kolei zrobiło zwykłego merge'a dodając commita do historii brancha.

PiotrWeppo commented 1 year ago

Czemu robisz pull requesta ze swojego brancha roboczego (na własnym repo) na innego brancha roboczego (na tym repo)?

Nie rozumiem. Gdybym nie zrobił pull requesta to byś nigdy nie zauważył, że wprowadziłem jakiekolwiek zmiany. Natomiast pull requesta zrobiłem tylko raz, wczoraj. Dzisiejsze commity automatycznie się tu pojawiają, przez to, że te branche na moim originie i upstreamie są ze sobą "związane".

pktiuk commented 1 year ago

Tu trzeba było uzyć interaktywnego rebase'a, a nie zwykłego git rebase -i.

PiotrWeppo commented 1 year ago

Tu trzeba było uzyć interaktywnego rebase'a, a nie zwykłego git rebase -i.

Czy można coś takiego zrobić w git graphie? Jedyne co znalazłem, to przy użyciu gitlensa: https://www.youtube.com/watch?v=3o_01F04bZ4

pktiuk commented 1 year ago

Hmm, mam zagwozdkę. Cały czas embed_creator i embed_methods mają błąd "circular imports". Gdybym je spakował razem to nadal moduł będzie miał poniżej 1000 wierszy (ok 750). Bardziej wolałbym przebudować kod, któregoś, ale nie wiem jak to ugryźć przy takim zaawansowaniu.

Na ten moment połączyłbym to w jeden plik i usiadł do tego przy kolejnym pull requeście. Ogólnie jest tutaj problem z niewłaściwym podziałem. bo patrząc na wizualizację zależności klas:

image

Widać, że do oddzielnego pliku przemieściłeś tak jakby coś co było w środku struktury, więc trzeba było przenieść klasę EmbedEditingMethods razem z klasami poniżej, albo nie przenosić wcale.

PiotrWeppo commented 1 year ago

Tu trzeba było uzyć interaktywnego rebase'a, a nie zwykłego git rebase -i.

już chyba ogarnąłem

PiotrWeppo commented 1 year ago

Na ten moment połączyłbym to w jeden plik i usiadł do tego przy kolejnym pull requeście. Ogólnie jest tutaj problem z niewłaściwym podziałem. bo patrząc na wizualizację zależności klas:

image

Widać, że do oddzielnego pliku przemieściłeś tak jakby coś co było w środku struktury, więc trzeba było przenieść klasę EmbedEditingMethods razem z klasami poniżej, albo nie przenosić wcale.

Z tym sobie też poradziłem.

PiotrWeppo commented 1 year ago

@pktiuk gotowe do review.

PiotrWeppo commented 1 year ago

Zapomniałeś zmienić nazwę skryptu do uruchomienia w Readme. Poza tym masz jeszcze kilka niezamkniętych wątków z komentarzami

Mógłbyś podać przykład? Przejrzałem wszystko i z grubsza jestem zadowolony z docstringów, które stworzyłem.