secure-systems-lab / lab-guidelines

How-to guides for Secure Systems Lab (SSL) projects and documents
https://ssl.engineering.nyu.edu/
MIT License
11 stars 8 forks source link

Add code review guideline #18

Closed lukpueh closed 5 years ago

lukpueh commented 5 years ago

Add guideline document about code reviews that may be used in addition to the development guideline document by code authors and code reviewers.

This document is largely based on a proposal by @aaaaalbert (see #17). It aims at making his proposal more succinct and adds post-review code author guidelines. Thanks, Albert!

I suggest @jhdalek55 and/or @JustinCappos as reviewers. Please review for conciseness and unnecessary redundancy with our development guidelines. Feel free to edit on this branch.

lukpueh commented 5 years ago

Note that the current version of the document does not include review examples, as suggested in #17. Although those might be helpful, I think the document is also valuable as is.

jhdalek55 commented 5 years ago

I will try to take a look later today.

jhdalek55 commented 5 years ago

Hey all. Sorry I have not checked in on this. We were scheduled to move last week and then didn't and that plus a bunch of other open commitments has slowed me down a bit. I will take a look either later today or tomorrow. Till then, it's nice seeing Albert's name pop up in my email. Hope all is well with you, Albert.

lukpueh commented 5 years ago

Thanks for the kind review, @aaaaalbert. I integrated your suggestions and tried to clarify a few things in 9e81129. Please, let me know what you think.

aaaaalbert commented 5 years ago

My pleasure ❤️ , let's do this interactively!

lukpueh commented 5 years ago

Thanks for the great word smithing, @jhdalek55, and for reviewing and merging, @JustinCappos! It seems that we are good to merge the entire document.