New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Article about Code Review #33

Merged
merged 3 commits into from Nov 5, 2017

Conversation

Projects
None yet
2 participants
@honzajavorek
Owner

honzajavorek commented Nov 5, 2017

Notes:

honzajavorek added some commits Nov 5, 2017

@honzajavorek honzajavorek merged commit 462b850 into master Nov 5, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@honzajavorek honzajavorek deleted the honzajavorek/code-review branch Nov 5, 2017

Zde záměrně smíchám to, co ve firmě děláme všichni, a to, co dělám já. Nebo co bych si přál, aby bylo běžné. Když přijdu k PR jako někdo, kdo má dělat review, postupuji následovně:
- Přečtu si popis PR. Pokud jej nemá nebo nevysvětluje _proč_ jsou změny prováděny, PR zavřu, nezajímá mě.

This comment has been minimized.

@tencek

tencek Nov 7, 2017

Please explain what "PR zavřu" means - it is not clear to me.
Different tools use different terminologies, but "approve" (schválit) and "decline" (zamítnout) is some sort of industry standard and clear to anybody. But the term "Close" can mean either letting the code to go to the master branch or the opposite.

@tencek

tencek Nov 7, 2017

Please explain what "PR zavřu" means - it is not clear to me.
Different tools use different terminologies, but "approve" (schválit) and "decline" (zamítnout) is some sort of industry standard and clear to anybody. But the term "Close" can mean either letting the code to go to the master branch or the opposite.

This comment has been minimized.

@honzajavorek

honzajavorek Nov 8, 2017

Owner

Asi můžeme klidně česky. Zavřením jsem myslel tlačítko Close. Dělali jsme to tak, než se GitHub naučil ty dnešní reviews. Dnes tedy už PR nezavíráme, asi jedině pokud jsou již překonané nebo zcela chybné. Stará terminologie mi ale očividně vlezla pod kůži. To, co popisuji, je spíše zamítnutí (s komentářem proč a s možností nápravy).

@honzajavorek

honzajavorek Nov 8, 2017

Owner

Asi můžeme klidně česky. Zavřením jsem myslel tlačítko Close. Dělali jsme to tak, než se GitHub naučil ty dnešní reviews. Dnes tedy už PR nezavíráme, asi jedině pokud jsou již překonané nebo zcela chybné. Stará terminologie mi ale očividně vlezla pod kůži. To, co popisuji, je spíše zamítnutí (s komentářem proč a s možností nápravy).

This comment has been minimized.

@honzajavorek

honzajavorek Nov 8, 2017

Owner

Díky za připomínku, opravil jsem to - #34 Jen pro upřesnění, co jsem přesně myslel tím tlačítkem Close:

screen shot 2017-11-08 at 20 34 53

@honzajavorek

honzajavorek Nov 8, 2017

Owner

Díky za připomínku, opravil jsem to - #34 Jen pro upřesnění, co jsem přesně myslel tím tlačítkem Close:

screen shot 2017-11-08 at 20 34 53

This comment has been minimized.

@tencek

tencek Nov 9, 2017

Už rozumím, díky.

@tencek

tencek Nov 9, 2017

Už rozumím, díky.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment