search

IQSS / social_science_software_toolkit

IQSS best practices and resources for developing social science statistical software
10 stars 1 forks source link

Code Review #4

Open christophergandrud opened 7 years ago

christophergandrud commented 7 years ago

The best practices currently do not say anything about code review. How would this be good to incorporate?

(@pdurbin any thoughts?)

pdurbin commented 7 years ago

Ah, you're saying https://github.com/IQSS/social_science_software_toolkit/blob/master/iqss_sss_best_practices.md doesn't say anything about code review. You're right. It probably should. 😄

As of this writing https://github.com/IQSS/dataverse/blob/develop/CONTRIBUTING.md#codepull-requests says

"After making your pull request, your goal should be to help it advance through our kanban board at https://waffle.io/IQSS/dataverse . If no one has moved your pull request to the code review column in a timely manner, please reach out. Thanks!"

I'm also reiterating it at https://github.com/IQSS/dataverse/blob/29d3e8fa708831539cdb876e598f938ae761ddc9/doc/sphinx-guides/source/developers/branching-strategy.rst with

"Your goal is have your code merged into the develop branch after it has been reviewed."

and

"Create a pull request based on the feature branch you pushed. As mentioned in https://github.com/IQSS/dataverse/blob/develop/CONTRIBUTING.md if you do not have access to advance your pull request into the "Code Review" column at https://waffle.io/IQSS/dataverse you should reach out to ask for it to be moved on your behalf."

I hope this helps. Obviously, it's based on Dataverse's use of a kanban board, but I'm not sure if this applies for the IQSS Social Science Software Toolkit.

christophergandrud commented 7 years ago

Thanks for the details @pdurbin!

I'm thinking for https://github.com/IQSS/social_science_software_toolkit/blob/master/iqss_sss_best_practices.md it should probably be even more general. Though this is a tricky one for best practices. In principle, I think stats software source code should be reviewed like peer reviewed journal articles.

However, this get's tricky to implement in practice given the overall low level of programming skills among many scientists, especially social scientists, and already high review demands. Maybe a more realistic expectation is that the tests should be reviewed. Not only should they pass, but an outside peer should review that they have reasonable expectations.