Hi Sean, On Tue, 6 Oct 2020 at 13:16, Sean Anderson <sean...@gmail.com> wrote: > > This adds several commands to add, list, and remove log filters. Due to the > complexity of adding a filter, `log filter-list` uses options instead of > positional arguments. > > These commands have been added as subcommands to log by using a dash to > join the subcommand and subsubcommand. This is stylistic, and they could be > converted to proper subsubcommands if it is wished. > > Signed-off-by: Sean Anderson <sean...@gmail.com> > --- > > cmd/Kconfig | 1 + > cmd/log.c | 213 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 214 insertions(+)
I see you have a lot of comments already, It's great to see this feature. But please do add some sort of test for the command itself. You don't need to test that the filters work; that sort of thing belongs in the log tests. But the command test should check that various commands are interpreted correctly, result in some action (you can check the log state directly if you provide a helper function to export data structures) and output things correctly (for commands that produce output). Regards, Simon
