[riot-devel] Pull request procedure

Oleg Hahm oliver.hahm at inria.fr
Fri Aug 21 11:35:05 CEST 2015


Hey!

> Say I write a complex piece of code, add comments where I find them
> necessary. As I wrote the complex beast, I find everything "very clear".
> So I open the PR, and  someone says "sorry I don't understand and
> documentation is missing".
> 
> So I improve documentation and try to clarify things until the reviewers
> are happy.

Fully agree.

> That's how we've been doing things as far as I can remember.

If I look at large pieces of our code, I have to admit: unfortunately not. I
can only speak for me, but I believe that others made the same mistake:
reviewed code, spent (wasted) a lot of time to understand it, and failed to
ask for (better) documentation.

> Now of what use is a rule requiring "documentation that makes things
> very clear"? What does "very clear" mean?

Am I mistaken or do you react somehow allergic to the word "rule" here? It's
more like a guideline in a sense that one should try the best you can to make
the reviewer's job easier. If you think your code is well documented and
understandable, fine, you're all set!

Actually, this "rule" (or however you wanna call it) is more a reminder to
both parties: for the pull requester to think about documenting the code and
for the reviewer to ask for this documentation if it takes too much time to
understand it. Of course, "well documented" and "very clear" are blurry,
subjective terms, but if everyone tries to implement her/his personal
understanding of these terms, we're already in a much better position.

Cheers,
Oleg
-- 
/* James M doesn't say fuck enough. */
        linux-2.4.3/net/core/netfilter.c
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.riot-os.org/pipermail/devel/attachments/20150821/41c3fa5b/attachment.sig>


More information about the devel mailing list