Hi Ivan, On 16.09.23 20:05, Ivan Orlov wrote: > On 9/16/23 19:22, Javier Carrasco wrote: >> Defining the 'len' variable inside the 'patten_buf' as unsigned >> makes it more consistent with its actual meaning and the rest of the >> size variables in the test. Moreover, this removes an implicit >> conversion in the fscanf function call. >> > > Considering the fact that the pattern buffer length can't be negative or > larger that 4096, I really don't think that it is a necessary change. > >> Additionally, remove the unused variable 'it' from the reset_ioctl test. >> > > Your patches should always contain only one logical change. If you, for > instance, remove redundant blank lines, combining it with something else > is fine, but otherwise you should split the changes up. > Removing an unused variable is actually removing a blank line from a logical point of view. Is an extra patch not overkill considering that it cannot affect the code behavior? >> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx> >> --- >> Defining the 'len' variable inside the 'patten_buf' as unsigned >> makes it more consistent with its actual meaning and the rest of the >> size variables in the test. Moreover, this removes an implicit >> conversion in the fscanf function call. >> >> Additionally, remove the unused variable 'it' from the reset_ioctl test. > > You don't need this text here. Usually it is the place for changelog > between patch versions if we have more than one version of the patch. > For instance, if you send a patch V2, it could look like this: > Sorry, this got merged from the cover letter by b4. I will be more careful next time, thanks! > Signed-off-by: ... > --- > V1 -> V2: > - Improve something > - Add something > > So, don't repeat the commit message here :) > > Anyway, great job! I believe this test could be enhanced in lots of > ways, so I look forward to seeing new patches related to it from you :) > > -- > Kind regards, > Ivan Orlov Thanks for your feedback and best regards, Javier Carrasco
