diff options
Diffstat (limited to 'doc/developer/workflow.rst')
| -rw-r--r-- | doc/developer/workflow.rst | 77 |
1 files changed, 53 insertions, 24 deletions
diff --git a/doc/developer/workflow.rst b/doc/developer/workflow.rst index 8ce3bdeeb2..6885a41e0f 100644 --- a/doc/developer/workflow.rst +++ b/doc/developer/workflow.rst @@ -203,7 +203,6 @@ Submitting Patches and Enhancements FRR accepts patches from two sources: -- Email (git format-patch) - GitHub pull request Contributors are highly encouraged to use GitHub's fork-and-PR workflow. It is @@ -228,29 +227,6 @@ summary of the included patches. The description should provide additional details that will help the reviewer to understand the context of the included patches. -Patch Submission via Mailing List ---------------------------------- - -As an alternative submission method, a patch can be mailed to the -development mailing list. Patches received on the mailing list will be -picked up by Patchwork and tested against the latest development branch. - -The recommended way to send the patch (or series of NN patches) to the -list is by using ``git send-email`` as follows (assuming they are the N -most recent commit(s) in your git history):: - - git send-email -NN --annotate --to=dev@lists.frrouting.org - -If your commits do not already contain a ``Signed-off-by`` line, then -use the following command to add it (after making sure you agree to the -Developer Certificate of Origin as outlined above):: - - git send-email -NN --annotate --signoff --to=dev@lists.frrouting.org - -Submitting multi-commit patches as a GitHub pull request is **strongly -encouraged** and increases the probability of your patch getting reviewed and -merged in a timely manner. - .. _license-for-contributions: License for Contributions @@ -377,6 +353,14 @@ After Submitting Your Changes - An author must never delete or manually dismiss someone else's comments or review. (A review may be overridden by agreement in the weekly technical meeting.) + - When you have addressed someone's review comments, please click the + "re-request review" button (in the top-right corner of the PR page, next + to the reviewer's name, an icon that looks like "reload") + - The responsibility for keeping a PR moving rests with the author at + least as long as there are either negative CI results or negative review + comments. If you forget to mark a review comment as addressed (by + clicking re-request review), the reviewer may very well not notice and + won't come back to your PR. - Automatically generated comments, e.g., those generated by CI systems, may be deleted by authors and others when such comments are not the most recent results from that automated comment source. @@ -459,6 +443,24 @@ Guidelines for code review code change is large enough/significant enough to warrant such a requirement. +For project members with merge permissions, the following patterns have +emerged: + +- a PR with any reviews requesting changes may not be merged. + +- a PR with any negative CI result may not be merged. + +- an open "yellow" review mark ("review requested, but not done") should be + given some time (a few days up to weeks, depending on the size of the PR), + but is not a merge blocker. + +- a "textbubble" review mark ("review comments, but not positive/negative") + should be read through but is not a merge blocker. + +- non-trivial PRs are generally given some time (again depending on the size) + for people to mark an interest in reviewing. Trivial PRs may be merged + immediately when CI is green. + Coding Practices & Style ======================== @@ -539,6 +541,28 @@ your new claim at the end of the list. * ... */ +Defensive coding requirements +----------------------------- + +In general, code submitted into FRR will be rejected if it uses unsafe +programming practices. While there is no enforced overall ruleset, the +following requirements have achieved consensus: + +- ``strcpy``, ``strcat`` and ``sprintf`` are inacceptable without exception. + Use ``strlcpy``, ``strlcat`` and ``snprintf`` instead. (Rationale: even if + you know the operation cannot overflow the buffer, a future code change may + inadvertedly introduce an overflow.) + +- buffer size arguments, particularly to ``strlcpy`` and ``snprintf``, must + use ``sizeof()`` whereever possible. Particularly, do not use a size + constant in these cases. (Rationale: changing a buffer to another size + constant may leave the write operations on a now-incorrect size limit.) + +Other than these specific rules, coding practices from the Linux kernel as +well as CERT or MISRA C guidelines may provide useful input on safe C code. +However, these rules are not applied as-is; some of them expressly collide +with established practice. + Code Formatting --------------- @@ -992,6 +1016,11 @@ Miscellaneous When in doubt, follow the guidelines in the Linux kernel style guide, or ask on the development mailing list / public Slack instance. +JSON Output +^^^^^^^^^^^ + +All JSON keys are to be camelCased, with no spaces. + .. _documentation: |