On 11/22/18 9:50 PM, Simon Glass wrote:
Hi Heinrich,
On Wed, 21 Nov 2018 at 07:35, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 11/18/18 4:16 PM, Simon Glass wrote:
This is an experimental check for adding new uclasses without a test.
I am not sure of the best way to add U-Boot-specific tests, although in this case, it would not fire on Linux.
Signed-off-by: Simon Glass sjg@chromium.org
scripts/checkpatch.pl | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 373094e59ef..2f9edb429d5 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3033,6 +3033,13 @@ sub process { "adding a line without newline at end of file\n" . $herecurr); }
# ask for a test if a new uclass ID is addedif ($realfile =~ /uclass-id.h/ && $line =~ /^\+/) {WARN("NEW_UCLASS","Possible new uclass - make sure to add a test in test/dm/<name>.c\n" . $herecurr);}# check we are in a valid source file C or perl if not then ignore this hunk next if ($realfile !~ /.(h|c|pl|dtsi|dts)$/);
I would prefer if this script were a verbatim copy of the Linux upstream. Joe Perches is continually working on it. So let's update to 4.20 instead.
That's fine, but is not the point of this patch.
Your patch is making updates harder?
Tests are needed for any new functionality. Nothing special about u-classes here. The reviewer/custodian should take care that this is observed.
We have similar checks for updating MAINTAINERS files when adding/removing files. I think it is better for people to find out that they need to add tests before they send their patches.
This is an upstream function.
Also we have 73 uclasses but not all have tests, so some have clearly slipped through the cracks.
This is not specific for u-classes. With the same reasoning you could warn on all patches creating a C function, "Did you create a test?" and for any other change, "Did you update the tests?".
I am with you in that we should aim for 100 % test coverage. But I don't like nag messages.
Regards
Heinrich
Regards, Simon