search

eddatasci / unrollment_proj

The Unrollment Project: Exploring algorithmic bias in predicting bachelor's degree completion.
5 stars 0 forks source link

code review #25

Open ozanj opened 4 years ago

ozanj commented 4 years ago

I spent some time learning about "pull requests" today.

I now understand the basics of what pull requests are, why they are important, and how to make them

However, the really important part of pull requests seem to be peer "code review." I feel clueless here. In watching youtube videos about pull requests I couldn't find much about what to actually do when reviewing code.

can you recommend a good tutorial or youtube video about how we are supposed to go about code review in the context of a pull request. something that is simultaneously conceptual and practical.

if there is no video out there that is digestible for social scientist who aren't coders, perhaps we should create one as part of the public facing part of the unrollment project?

btskinner commented 4 years ago

Can you link one or two of the better tutorials or videos you've seen so far?

My guess is that there probably isn't much information directed at social scientists, so a better guide could be valuable. I'm not opposed to having one come from this project as a secondary product down the road.

bradweiner commented 4 years ago

Greetings all. Long time listener, first time caller. It's been fun watching progress so far and I look forward to the final result.

@ozanj this is a great question.

Here is a set of guidelines that we've used in the past. https://github.com/google/eng-practices/blob/master/review/reviewer/looking-for.md

There's also a decent quick read here written for data science work which might be worth thinking about separately than a small piece of code that needs to fit into a larger piece of production-ready software like the previous link generated by Google engineers. https://medium.com/apteo/code-reviewing-data-science-work-774747248e33

Good luck!

ozanj commented 4 years ago

Thank you @bradweiner !!!

@btskinner and/or @wdoyle42 - should we include these links anywhere on unrollment_proj repo for the public-facing aspect of this project or should I just close the issue?

@cyouh95 and @mpatricia01 - see the links that Brad gave us to answer my question about guidelines for code review (#25). [we should use this for teaching]

wdoyle42 commented 4 years ago

Thank you @bradweiner!

Yes, let's include these.