Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<!--
Thanks for contributing to bpfilter.

Before opening this PR: you should understand and stand behind every line in it,
and you should have built and tested it with the real workflow, not a one-off
compile. If you used AI for any of it, that's fine, just say so below. See
https://bpfilter.io/developers/contributing.html for the details.

Fill in the sections below and remove the irrelevant ones.
-->

## Summary

<!-- What does this change do, and why? Keep it to the point. -->

## Related issue

<!-- e.g. "Fixes #123". If there is no issue, explain why the change is needed. -->

Fixes #

## Testing

<!--
How did you verify this? Describe the build and tests you actually ran, not what
you expect CI to do. Configure and build with CMake as the developer docs
describe, run `make -C $BUILD test_bin test`, and `make -C $BUILD doc` if you
changed the documentation.

If this is one of your first PRs here, paste the relevant test output below. The
summary line and pass counts are enough.
-->

## Notes for the reviewer

<!--
Optional. Anything I should know: open questions, deliberate scope limits,
tradeoffs you made, or areas you want feedback on. Flagging these here saves a
round trip.
-->

## AI disclosure

<!--
First, read https://bpfilter.io/developers/contributing.html.

Did you use AI for any part of this (code, tests, this description)? Say so and
roughly for what. "None" is a fine answer. Undisclosed AI is grounds for closing
the PR.
-->

## Checklist

- [ ] I understand every line in this PR and can explain why it is correct
- [ ] I built the project and ran the tests covering this change
- [ ] `make -C $BUILD test_bin test` passes and the code follows the style guide
- [ ] Commits are formatted as `component: subcomponent: short description`
- [ ] I have disclosed any AI usage above
28 changes: 1 addition & 27 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,29 +1,3 @@
# Contributing to bpfilter

We want to make contributing to this project as easy and transparent as possible.

## Pull Requests

We actively welcome your pull requests.

1. Fork the repo and create your branch from `main`.
2. If you've added code that should be tested, add tests.
3. If you've changed APIs, update the documentation.
4. Ensure the test suite passes.
5. If you haven't already, complete the Contributor License Agreement ("CLA").

## Contributor License Agreement ("CLA")

In order to accept your pull request, you need to submit a CLA. You only need to do this once to work on any of Meta's open source projects.

Complete your CLA here: <https://code.facebook.com/cla>

## Issues

We use GitHub issues to track public bugs. Please ensure your description is clear and has sufficient instructions to be able to reproduce the issue.

Meta has a [bounty program](https://www.facebook.com/whitehat) for the safe disclosure of security bugs. In those cases, please go through the process outlined in that page and do not file a public issue.

## License

By contributing to bpfilter, you agree that your contributions will be licensed under the LICENSE file in the root directory of this source tree.
See <https://bpfilter.io/developers/contributing.html>.
59 changes: 49 additions & 10 deletions doc/developers/contributing.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,34 @@
Contributing
============

Using AI to contribute to bpfilter
----------------------------------

.. admonition:: Why am I reading this?

One of two reasons. Either you're thinking about contributing and want to know how AI fits in here, in which case read on, it's short. Or your PR was just closed with a link to this page, in which case the change looked like nobody wrote, understood, or tested it before sending it for review, or you used AI without saying so. Same document either way.

We have nothing against AI. We use it, and so do the regular contributors to this project. Autocomplete, LLMs, coding agents, all fine. You don't need to hide it or avoid it.

What we care about is time. Reviewing a change costs the reviewers time, and we're glad to spend it when the person who opened the PR spent theirs first: writing the code, understanding it, or testing it. When a PR is a prompt result its author never read, that trade is broken. You spent a few seconds and you're asking a reviewer for an hour. We don't care what you used. We care whether you did the work.

So before you open a PR:

**Understand what you're submitting.** You should be able to explain every line, why it's correct, and what you ruled out. If you can't review your own diff, don't send it. We won't do that review for you.

**Build and test it properly.** Configure and build with CMake the way the developer docs describe, run ``make -C $BUILD test_bin test`` (``make -C $BUILD fixstyle`` formats the code for you), and ``make -C $BUILD doc`` if you changed the documentation. The tests that cover your change matter most: the cmocka unit tests under ``tests/unit/``, plus the e2e suite where it applies. "I changed the code to fix X but didn't build or run it" is not a testing plan. If you're not a regular here yet, paste the test output into the PR. The summary line and pass counts are enough, and it's the fastest way to show us you actually ran it.

**Follow the style guide.** Formatting is enforced by ``.clang-format``, the rest is in :doc:`style`, and commits use ``component: subcomponent: short description``. Ignoring the style after it's been pointed out tells us the same thing an untested diff does: nobody looked.

**Say if you used AI.** A line in the PR is enough ("AI-drafted the tests and description, wrote the code by hand"). "None" is a fine answer. We may ask, so just be straight about it. Undisclosed AI is one of the things that gets a PR closed with a link to this page.

We close PRs, without a full review, when they were clearly never read by their author, weren't tested, or ignore the style guide after a heads-up. If it keeps happening we'll block you. We've spent more time on some of these review comments than the author spent opening the PR, and we won't keep making that trade.

If you're new and willing to put the work in, you're welcome here and we'll take the time to ramp you up. Pick something tagged `good first issue <https://github.com/facebook/bpfilter/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22>`_, or email us (qde@naccy.de) and we'll find you a good starting task.

Where to start
--------------

If you want to start contributing to bpfilter, the best way to get to know the codebase would be to start with one of the ``@todo`` available in the code. Most of those tasks are small, self-contained, work trivial enough that they do not deserve their GitHub issue.

Once you know your way around the structure of the project, feel free to continue with the ``@todo``, or jump on a bigger issue in the `GitHub issues tracker <https://github.com/facebook/bpfilter/issues>`_.
Expand All @@ -9,17 +37,28 @@ You are welcome to reach out to qde@naccy.de if you need help, or have any quest


To do
-----
~~~~~

* Remove the RPM ``x86_64`` macro `workaround <https://pagure.io/epel/issue/325>`_ from the Fedora ``bpfilter.spec``.
* Gate the documentation generate in Fedora's ``bpfilter.spec`` with a ``bcond``.
* Add support for CMake 4.0 and ``ninja``.
* Handle extra characters in the lexer (currently, any non-matched token will be printed to ``stdout``).
* Add support for missing matcher operators (e.g. ``meta.l4_proto not``).
* Add a Fedora 43 build in the CI.
.. doxygenpage:: todo


From the code
~~~~~~~~~~~~~
Contributor License Agreement ("CLA")
-------------------------------------

.. doxygenpage:: todo
In order to accept your pull request, you need to submit a CLA. You only need to do this once to work on any of Meta's open source projects.

Complete your CLA here: https://code.facebook.com/cla


Security
--------

We use GitHub issues to track public bugs. Please ensure your description is clear and has sufficient instructions to be able to reproduce the issue.

Meta has a `bounty program <https://www.facebook.com/whitehat>`_ for the safe disclosure of security bugs. In those cases, please go through the process outlined in that page and do not file a public issue.


License
-------

By contributing to bpfilter, you agree that your contributions will be licensed under the ``LICENSE`` file in the root directory of this source tree.
Loading