Skip to content

docs: README#1

Open
necco-c wants to merge 1 commit into
mainfrom
readme/initial
Open

docs: README#1
necco-c wants to merge 1 commit into
mainfrom
readme/initial

Conversation

@necco-c
Copy link
Copy Markdown

@necco-c necco-c commented Jun 4, 2026

Reviewer notes for grpcsnoop

✅ Reviewer action items

  • Apply the About description — paste-ready line in the About description section below.
  • Apply the topic tags — paste-ready code block in the Topic tags section below.
  • Confirm the Discord badge URL — the badge at the top of the README links to discord.gg/bBRF9d2Gw2, which is different from the canonical yeet invite (discord.gg/dYZu9PjKB) used in the new footer. If the badge is intentionally a separate channel (grpcsnoop-specific?), fine; otherwise unify it with the footer link.
  • Resolve flagged claims — two grounding-≥-2 flags inline in the README. Strike the (extrapolated, grounding N — review) tags from the README in the same commit.
    • Confirm kernel ≥ 6.6 floor for tcx (the existing README also stated 6.6; consistent)
    • Confirm the JS-side license framing (or address it via the LICENSE-file action item below)
  • Add a LICENSE filegrpcsnoop.bpf.c declares SPDX-License-Identifier: GPL-2.0 and SEC("license") = "GPL", but the repo has no top-level LICENSE file and the JS modules have no license headers. Pick a license and add the file.
  • Update the Quick start to genuinely start with the shorthand — the new draft puts yeet run github:yeet-src/grpcsnoop -- --port 50051 first, then offers git clone + bash demo/up.sh as the second path for trying the bundled demo. The old README led with make and ran yeet run . --port 50051 against the demo. Confirm the new ordering (shorthand first, demo second) is right.

About description

tcpdump for gRPC. Decodes protobuf messages between services live, from the kernel. Plaintext only.

(101 chars)

Topic tags

ebpf bpf linux grpc protobuf http2 tcx tc observability debugging tracing containers networking yeet kernel

The obvious-search cluster is ebpf, bpf, linux, grpc, protobuf, http2, observability. tcx and tc carry the specific kernel-attach-point language (and match what upstreamtop ships, for consistency). containers is genuinely load-bearing here because the tool's whole premise is east-west container traffic. yeet and kernel carry the corpus-wide language.

Flagged for review (grounding ≥ 2)

  • Requirements / kernel ≥ 6.6 — grounding 2. The existing README also states "Linux ≥ 6.6 (TCX links)". tcx attach via BPF_LINK_TYPE_TCX merged in 6.6, so the floor is honest. Source code itself doesn't gate on a version. Safe to ship.
  • License / JS side undeclared — grounding 2. grpcsnoop.bpf.c declares SPDX-License-Identifier: GPL-2.0 and SEC("license") = "GPL". The repo has no top-level LICENSE file and the JS modules have no license headers. Stating that "the JS side has no separate license declaration in the source" is consistent with the code state; cleanest resolution is to add a LICENSE file (see action item above).

Source-level cleanup worth resolving in the repo (not in the README)

  • No LICENSE file. BPF is GPL by source declaration; JS side is unstated. Same pattern as ciprof, upstreamtop, poolnarc — worth a corpus-wide pass at some point.
  • Makefile defaults to sudo bpftool for the vmlinux.h generation (line 4). That's the build-time bpftool, separate from the yeet run flow, but worth flagging because the template's "no sudo yeet examples" rule generalizes loosely to "the README shouldn't show sudo unless genuinely required" — and the Quick start now omits the make step entirely (uses github: shorthand), so the sudo is hidden from the happy path. Good. The make path inside the "Building from source" section keeps it.
  • grpcsnoop.bpf.c is SPDX-License-Identifier: GPL-2.0 (header), while SEC("license") = "GPL" is the runtime declaration. Both are present and consistent.

Template-level observations

Possible new mode worth promoting: "medium-touch." This run sat between the default full-rewrite mode (used for ciprof, upstreamtop, the first upstreamtop pass) and the light-touch mode (used for poolnarc when explicitly requested). The existing grpcsnoop README was strong enough that a full rewrite would have destroyed value, but it wasn't explicitly flagged as "preserve content."

The pattern that emerged: when the existing README already passes template guards (one-sentence definition ≤ 25 words, has a 60-second primer, has honest caveats, has community FAQ, has a tagline), default to medium-touch: preserve substance, normalize structure to template order, swap installation/footer to the new conventions, tighten in-sentence em-dashes.

Worth a template bump bullet under "Preferences captured from prior reviews": something like "When the existing README already covers the template's mandatory sections with substantive content, default to medium-touch — preserve the prose inside each section, reorder to template order, swap installation/footer to current conventions, tighten in-sentence em-dashes. Don't full-rewrite a strong README just because the default mode is full-rewrite."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant