| Message ID | 20200904161454.31135-1-quentin@isovalent.com |
|---|---|
| Headers | show |
| Series | bpf: format fixes for BPF helpers and bpftool documentation | expand |
On Fri, Sep 4, 2020 at 9:15 AM Quentin Monnet <quentin@isovalent.com> wrote: > > This series contains minor fixes (or harmonisation edits) for the > bpftool-link documentation (first patch) and BPF helpers documentation > (last two patches), so that all related man pages can build without errors. > > Quentin Monnet (3): > tools: bpftool: fix formatting in bpftool-link documentation > bpf: fix formatting in documentation for BPF helpers > tools, bpf: synchronise BPF UAPI header with tools > > include/uapi/linux/bpf.h | 87 ++++++++++--------- > .../bpftool/Documentation/bpftool-link.rst | 2 +- > tools/include/uapi/linux/bpf.h | 87 ++++++++++--------- > 3 files changed, 91 insertions(+), 85 deletions(-) > > -- > 2.20.1 > This obviously looks good to me: Acked-by: Andrii Nakryiko <andriin@fb.com> But do you think we can somehow prevent issues like this? Consider adding building/testing of documentation to selftests or something. Not sure if that will catch all the issues you've fixed, but that would be a good start.
On 9/4/20 6:14 PM, Quentin Monnet wrote: > This series contains minor fixes (or harmonisation edits) for the > bpftool-link documentation (first patch) and BPF helpers documentation > (last two patches), so that all related man pages can build without errors. > > Quentin Monnet (3): > tools: bpftool: fix formatting in bpftool-link documentation > bpf: fix formatting in documentation for BPF helpers > tools, bpf: synchronise BPF UAPI header with tools > > include/uapi/linux/bpf.h | 87 ++++++++++--------- > .../bpftool/Documentation/bpftool-link.rst | 2 +- > tools/include/uapi/linux/bpf.h | 87 ++++++++++--------- > 3 files changed, 91 insertions(+), 85 deletions(-) > Applied, thanks!
On 04/09/2020 22:40, Andrii Nakryiko wrote: > On Fri, Sep 4, 2020 at 9:15 AM Quentin Monnet <quentin@isovalent.com> wrote: >> >> This series contains minor fixes (or harmonisation edits) for the >> bpftool-link documentation (first patch) and BPF helpers documentation >> (last two patches), so that all related man pages can build without errors. >> >> Quentin Monnet (3): >> tools: bpftool: fix formatting in bpftool-link documentation >> bpf: fix formatting in documentation for BPF helpers >> tools, bpf: synchronise BPF UAPI header with tools >> >> include/uapi/linux/bpf.h | 87 ++++++++++--------- >> .../bpftool/Documentation/bpftool-link.rst | 2 +- >> tools/include/uapi/linux/bpf.h | 87 ++++++++++--------- >> 3 files changed, 91 insertions(+), 85 deletions(-) >> >> -- >> 2.20.1 >> > > This obviously looks good to me: > > Acked-by: Andrii Nakryiko <andriin@fb.com> > > But do you think we can somehow prevent issues like this? Consider > adding building/testing of documentation to selftests or something. > Not sure if that will catch all the issues you've fixed, but that > would be a good start. > Thanks for the review. As for preventing future issues, I see two cases. Some minor fixes done to harmonise the look of the description for the different helpers could be checked with some kind of dedicated checkpatch-like script that would validate new helpers I suppose, but I'm not sure whether that's worth the trouble of creating that script, creating rules and then enforcing them. The issues that do raise warnings are more important to fix, and easier to detect. We could simply build bpftool's documentation (which also happens to build the doc for eBPF helpers) and checks for warnings. We already have a script to test bpftool build in the selftests, so I can add it there as a follow-up and make doc build fail on warnings. On a somewhat related note I also started to work on a script to check that bpftool is correctly sync'ed (with kernel regarding prog names / map names etc., and between source code / doc / bash completion) but I haven't found the time to finish that work yet. Quentin
On Mon, Sep 7, 2020 at 7:48 AM Quentin Monnet <quentin@isovalent.com> wrote: > > On 04/09/2020 22:40, Andrii Nakryiko wrote: > > On Fri, Sep 4, 2020 at 9:15 AM Quentin Monnet <quentin@isovalent.com> wrote: > >> > >> This series contains minor fixes (or harmonisation edits) for the > >> bpftool-link documentation (first patch) and BPF helpers documentation > >> (last two patches), so that all related man pages can build without errors. > >> > >> Quentin Monnet (3): > >> tools: bpftool: fix formatting in bpftool-link documentation > >> bpf: fix formatting in documentation for BPF helpers > >> tools, bpf: synchronise BPF UAPI header with tools > >> > >> include/uapi/linux/bpf.h | 87 ++++++++++--------- > >> .../bpftool/Documentation/bpftool-link.rst | 2 +- > >> tools/include/uapi/linux/bpf.h | 87 ++++++++++--------- > >> 3 files changed, 91 insertions(+), 85 deletions(-) > >> > >> -- > >> 2.20.1 > >> > > > > This obviously looks good to me: > > > > Acked-by: Andrii Nakryiko <andriin@fb.com> > > > > But do you think we can somehow prevent issues like this? Consider > > adding building/testing of documentation to selftests or something. > > Not sure if that will catch all the issues you've fixed, but that > > would be a good start. > > > > Thanks for the review. > > As for preventing future issues, I see two cases. Some minor fixes done > to harmonise the look of the description for the different helpers could > be checked with some kind of dedicated checkpatch-like script that would > validate new helpers I suppose, but I'm not sure whether that's worth > the trouble of creating that script, creating rules and then enforcing them. > > The issues that do raise warnings are more important to fix, and easier > to detect. We could simply build bpftool's documentation (which also > happens to build the doc for eBPF helpers) and checks for warnings. We > already have a script to test bpftool build in the selftests, so I can > add it there as a follow-up and make doc build fail on warnings. Yeah, that would be good. I constantly forget to try building documentation for bpftool, so having this automated would be an improvement (provided building docs doesn't require unreasonable dependencies). > > On a somewhat related note I also started to work on a script to check > that bpftool is correctly sync'ed (with kernel regarding prog names / > map names etc., and between source code / doc / bash completion) but I > haven't found the time to finish that work yet. > > Quentin