code review about neuralmonkey HOT 14 CLOSED

I don't know, I am usually against these kinds of corporate coding. It is way too much overhead to the work actually done. I think for this small project that nobody uses so far, it is pointless.

from neuralmonkey.

It should create almost no overhead compared to the way we should do things now. You can work as fast and dirty as you wish, if you do that in a separate branch. When you are merging into master, you should polish your code and discuss important changes and design decisions before merging.

Using a code-review tool would just make the last stage easier and more organized than a GitHub PR single-thread discussion.

from neuralmonkey.

Do you have experience with such tools? Reviewable has a terrible corporate website telling me that a senior PHP developer thinks it is a miraculous tools. This is an evident sign, we should not use it. I am not sure if we need such a thing, but if yes I would prefer something open-source, e.g. barkeep?

from neuralmonkey.

We need GitHub integration (otherwise there is no point in this) so if you want open-source, we can use

• pullaprove or
• rewiev.ninja

Both seem to have the same features, so I don't really care which one we choose, but I think we should use one.

from neuralmonkey.

uvědomuješ si, že na tom budem dělat jen my dva a že já netušim, co vlastně děláš ty a ty zas tvrdíš, že nechápeš, co dělá ten zbytek, takže je to celý zbytečný? Code review je od toho, aby se do hotovýho projektu na kterým dělá velkej tým a kterej běží někde na produkci, nedostaly nový bugy. Tohle neni ani hotovej projekt, ani velkej tým, ani produkční prostředí.

from neuralmonkey.

to je stejná blbost jako psát všechny issues anglicky, přestože jsme jediný lidi, co to tady čtou.

from neuralmonkey.

Ještě to čte Martin Popel ;-)

from neuralmonkey.

• Right now, I'm mostly just refactoring things, to have a maintainable package for the MTM project.
• The fact that I don't understand the design (if there was any) behind most current codebase does not mean that I cannot review new code.
• This isn't a production environment, but we have a (somewhat) working version in master and someone (who might it be, I wonder) is introducing bugs into it (eg. #20, #21). Code reviews (even informal) would prevent that.
• You're right that we don't need a new tool when just two people will be working on the repo (I didn't know that this would be the case when I opened this issue.).
• But from three people up I think it can be useful. I'd like to clearly see whether someone else beside the author read the content of a pull request and approved it. One of the tools mentioned above would help with that.
• Also, if we have these processes in place now, we will be ready for MTM, where we hope to have many people working on it at once. That's also the reason to write everything in English, so when someone asks "What is the design behind configuration and why?" at MTM, I can point them towards the corresponding issue on GitHub and don't have to explain everything.

from neuralmonkey.

Ad 3: Pochybuju, že by review těchto dvou issues pomohlo. V jednom případě byla chyba někde jinde, než změny, takže kdyby to někdo po mě četl, tak by mu to taky nedošlo a v druhym případě těžko říct, jestli by reviewera napadlo, že to funguje jen s absolutníma cestama. Každopádně k odhalování zmíněných bugů neni review ale testy, takže příklady, kterýs dal, nejsou validní.

Ad 4: wontfix & close

Ad 5: Nemám s tim takovou zkušenost. Moje zkušenosti jsou takový, že přes ty reviews i tak projde velká část bugů a že ve výsledku to zpomaluje postup víc, než to pomáhá.

Ad 6: Doufám, že tyhle naše blitky nebudou tou dobou jediná dokumentace toho, co bude hotový. Navíc se tu podle mě řeší spíš blbosti než něco, na co by se někdo někdy ptal.

from neuralmonkey.

Ad 5: zpomaluje to postup z větví do masteru, který má být pomalý, takže to je jedině dobře.

from neuralmonkey.

Píšu, že ve výsledku to škodí víc než pomáhá. Rozhodně nepíšu, že to zpomaluje postup z větví do masteru, píšu že to zpomaluje postup celkově.

from neuralmonkey.

A já s tím nesouhlasím, protože si myslím, že postup se má odehrávat ve větvých (takže na něm se nic nezpomaluje) a code review se děje před mergem z větve zpátky do masteru, což rozhodně nemá být unáhlená akce, takže tam je zpomalení naopak žádané. Tvůj celkový postup to krátkodobě může zpomalit jedině tím, že budeš muset číst něco, co napsal někdo jiný, aby to mohl mergovat. Ale tohle drobné zdržení se Ti bohatě vrátí na tom, že se do masteru nedostanou věci, které by Tě později zdržely mnohem víc.

from neuralmonkey.

Rozumim. Já dál ale tvrdim, že se tam ty věci dostanou tak jako tak.

from neuralmonkey.

Přinejmenším by se tam nedostaly některé nesrozumitelné věci, které tam teď jsou; že je něco napsané nesrozumitelně, na to se při code-review určitě přijde.

Nicméně shodneme se na tom, že pokud na tom v nejbližší době budeme pracovat jen dva, tak je zbytečné o tom dál diskutovat.

from neuralmonkey.