Pierre LALET | af2a96b | 2016-02-09 13:38:04 +0100 | [diff] [blame] | 1 | # How to contribute |
| 2 | |
| 3 | Contributors are essential to Scapy (as they are to most open source |
| 4 | projects). Here is some advice to help you help the project! |
| 5 | |
| 6 | ## Project objectives |
| 7 | |
| 8 | We try to keep Scapy as powerful as possible, to support as many |
| 9 | protocols and platforms as possible, to keep and make the code (and |
| 10 | the commit history) as clean as possible. |
| 11 | |
| 12 | Since Scapy can be slow and memory consuming, we try to limit CPU and |
| 13 | memory usage, particularly in parts of the code often called. |
| 14 | |
Pierre LALET | 94bfc64 | 2016-03-20 15:04:27 +0100 | [diff] [blame] | 15 | ## What to contribute? |
| 16 | |
| 17 | You want to spend to time working on Scapy but have no (or little) |
| 18 | idea what to do? You can look for open issues |
gpotter2 | ee4b5a3 | 2016-12-19 19:38:46 +0100 | [diff] [blame] | 19 | [labeled "contributions wanted"](https://github.com/secdev/scapy/labels/contributions%20wanted), or look at the [contributions roadmap](https://github.com/secdev/scapy/issues/399) |
Pierre LALET | 94bfc64 | 2016-03-20 15:04:27 +0100 | [diff] [blame] | 20 | |
| 21 | If you have any ideas of useful contributions that you cannot (or do |
| 22 | not want to) do yourself, open an issue and use the label |
| 23 | "contributions wanted". |
| 24 | |
| 25 | Once you have chosen a contribution, open an issue to let other people |
| 26 | know you're working on it (or assign the existing issue to yourself) |
| 27 | and track your progress. You might want to ask whether you're working |
| 28 | in an appropriate direction, to avoid the frustration of seeing your |
| 29 | contribution rejected after a lot of work. |
| 30 | |
Pierre LALET | af2a96b | 2016-02-09 13:38:04 +0100 | [diff] [blame] | 31 | ## Reporting issues |
| 32 | |
| 33 | ### Questions |
| 34 | |
| 35 | It is OK so submit issues to ask questions (more than OK, |
| 36 | encouraged). There is a label "question" that you can use for that. |
| 37 | |
| 38 | ### Bugs |
| 39 | |
| 40 | If you have installed Scapy through a package manager (from your Linux |
| 41 | or BSD system, from PyPI, etc.), please get and install the current |
| 42 | development code, and check that the bug still exists before |
| 43 | submitting an issue. |
| 44 | |
| 45 | Please label your issues "bug". |
| 46 | |
| 47 | If you're not sure whether a behavior is a bug or not, submit an issue |
| 48 | and ask, don't be shy! |
| 49 | |
| 50 | ### Enhancements / feature requests |
| 51 | |
| 52 | If you want a feature in Scapy, but cannot implement it yourself or |
| 53 | want some hints on how to do that, open an issue with label |
| 54 | "enhancement". |
| 55 | |
| 56 | Explain if possible the API you would like to have (e.g., give examples |
| 57 | of function calls, packet creations, etc.). |
| 58 | |
| 59 | ## Submitting pull requests |
| 60 | |
| 61 | ### Coding style & conventions |
| 62 | |
| 63 | First, Scapy "legacy" code contains a lot of code that do not comply |
| 64 | with the following recommendations, but we try to comply with the some |
| 65 | guidelines for new code. |
| 66 | |
| 67 | - The code should be PEP-8 compliant; you can check your code with |
| 68 | [pep8](https://pypi.python.org/pypi/pep8). |
| 69 | - [Pylint](http://www.pylint.org/) can help you write good Python |
| 70 | code (even if respecting Pylint rules is sometimes either too hard |
| 71 | or even undesirable; human brain needed!). |
| 72 | - [Google Python Style Guide](https://google.github.io/styleguide/pyguide.html) |
| 73 | is a nice read! |
| 74 | - Avoid creating unnecessary `list` objects, particularly if they |
gpotter2 | 22a55b6 | 2017-06-27 18:31:14 +0200 | [diff] [blame] | 75 | can be huge (e.g., when possible, use `scapy.modules.six.range()` instead of |
Pierre LALET | af2a96b | 2016-02-09 13:38:04 +0100 | [diff] [blame] | 76 | `range()`, `for line in fdesc` instead of `for line in |
| 77 | fdesc.readlines()`; more generally prefer generators over lists). |
| 78 | |
| 79 | ### Tests |
| 80 | |
| 81 | Please consider adding tests for your new features or that trigger the |
| 82 | bug you are fixing. This will prevent a regression from being |
| 83 | unnoticed. |
| 84 | |
| 85 | ### New protocols |
| 86 | |
| 87 | New protocols can go either in `scapy/layers` or to |
| 88 | `scapy/contrib`. Protocols in `scapy/layers` should be usually found |
| 89 | on common networks, while protocols in `scapy/contrib` should be |
| 90 | uncommon or specific. |
| 91 | |
| 92 | ### Features |
| 93 | |
| 94 | Protocol-related features should be implemented within the same module |
| 95 | as the protocol layers(s) (e.g., `traceroute()` is implemented in |
| 96 | `scapy/layers/inet.py`). |
| 97 | |
| 98 | Other features may be implemented in a module (`scapy/modules`) or a |
| 99 | contribution (`scapy/contrib`). |
| 100 | |
| 101 | ### Core |
| 102 | |
| 103 | If you contribute to Scapy's core (e.g., `scapy/base_classes.py`, |
| 104 | `scapy/packet.py`, etc.), please be very careful with performances and |
| 105 | memory footprint, as it is easy to write Python code that wastes |
| 106 | memory or CPU cycles. |
| 107 | |
| 108 | As an example, Packet().__init__() is called each time a **layer** is |
| 109 | parsed from a string (during a network capture or a PCAP file |
| 110 | read). Adding inefficient code here will have a disastrous effect on |
| 111 | Scapy's performances. |
| 112 | |
gpotter2 | 79675aa | 2017-03-22 17:40:33 +0100 | [diff] [blame] | 113 | ### Python 2 and 3 compatibility |
| 114 | |
| 115 | The project aims to provide code that works both on Python 2 and Python 3. Therefore, some rules need to be apply to achieve compatibility: |
gpotter2 | 72aa7b6 | 2017-05-02 15:35:52 +0200 | [diff] [blame] | 116 | - byte-string must be defined as `b"\x00\x01\x02"` |
| 117 | - exceptions must comply with the new Python 3 format: `except SomeError as e:` |
| 118 | - lambdas must be written using a single argument when using tuples: use `lambda x_y: x_y[0] + f(x_y[1])` instead of `lambda (x, y): x + f(y)`. |
gpotter2 | 22a55b6 | 2017-06-27 18:31:14 +0200 | [diff] [blame] | 119 | - use int instead of long |
| 120 | - use list comprehension instead of map() and filter() |
| 121 | - use scapy.modules.six.range instead of xrange and range |
| 122 | - use scapy.modules.six.itervalues(dict) instead of dict.values() or dict.itervalues() |
| 123 | - use scapy.modules.six.string_types instead of basestring |
gpotter2 | db9615b | 2017-06-27 23:20:49 +0200 | [diff] [blame] | 124 | - `__bool__ = __nonzero__` must be used when declaring `__nonzero__` methods |
| 125 | - `io.BytesIO` must be used instead of `StringIO` when using bytes |
| 126 | - `__cmp__` must not be used. |
| 127 | - UserDict should be imported via `six.UserDict` |
gpotter2 | 79675aa | 2017-03-22 17:40:33 +0100 | [diff] [blame] | 128 | |
Pierre LALET | af2a96b | 2016-02-09 13:38:04 +0100 | [diff] [blame] | 129 | ### Code review |
| 130 | |
| 131 | Maintainers tend to be picky, and you might feel frustrated that your |
| 132 | code (which is perfectly working in your use case) is not merged |
| 133 | faster. |
| 134 | |
| 135 | Please don't be offended, and keep in mind that maintainers are |
| 136 | concerned about code maintainability and readability, commit history |
| 137 | (we use the history a lot, for example to find regressions or |
| 138 | understand why certain decisions have been made), performances, |
| 139 | integration in Scapy, API consistency (so that someone who knows how |
| 140 | to use Scapy will know how to use your code), etc. |
| 141 | |
| 142 | **Thanks for reading, happy hacking!** |