[riot-notifications] [RIOT-OS/RIOT] gcoap: don't call random_uint32_range() when COAP_ACK_VARIANCE=0 (#11784)

Ken Bannister notifications at github.com
Wed Jul 31 04:15:32 CEST 2019


kb2ma commented on this pull request.

I understand the need for conditional compilation here, and the code tests fine. However, there is a larger issue here related to the first question in the Maintainer Guidelines. This question also applies to #11787, so we might as well discuss it here for both PRs.

> Does the reasoning for this PR make sense?

To me, "make sense" includes whether a user in a non-experimental setup would need this PR. It seems like if we open up the mainline code to what is needed for experiments, then that significantly increases the scope of what is acceptable. If it's just needed for an experiment, then why not just create a branch with the modifications for that purpose. Do we have a policy here?

At any rate, I think this PR is marginally acceptable for my "non-experimental" test. OTOH, it's hard to imagine how #11787 would be useful in a non-experimental setup.

I don't feel super strongly about this, but I hate to reduce readability in mainline code with conditionals that only a single experimental setup will ever need.



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/11784#pullrequestreview-268754882
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190730/90d9844f/attachment.htm>


More information about the notifications mailing list