Hi Brian, On Mon, 30 Sept 2024 at 08:47, Brian Ruley <brian.ru...@gehealthcare.com> wrote: > > On Mon, Sep 30, 2024 at 08:10:49AM -0600, Simon Glass wrote: > > > > WARNING: This email originated from outside of GE HealthCare. Please > > validate the sender's email address before clicking on links or attachments > > as they may not be safe. > > > > Hi Brian, > > > > On Mon, 30 Sept 2024 at 07:01, Brian Ruley <brian.ru...@gehealthcare.com> > > wrote: > > > > > > On Sun, Sep 29, 2024 at 06:46:23PM -0600, Tom Rini wrote: > > > > On Sun, Sep 29, 2024 at 04:49:17PM -0600, Simon Glass wrote: > > > > > Hi Fabio, > > > > > > > > > > On Sun, 29 Sept 2024 at 14:53, Fabio Estevam <feste...@gmail.com> > > > > > wrote: > > > > > > > > > > > > Hi Simon, Marek, and Tom, > > > > > > > > > > > > On Fri, Sep 27, 2024 at 5:47???PM Tom Rini <tr...@konsulko.com> > > > > > > wrote: > > > > > > > > > > > > > > Please can you coordinate with Marek as we need to sort out the > > > > > > > > test > > > > > > > > coverage for this etype, before adding more functionality. I > > > > > > > > did a > > > > > > > > starting point, now in -next, which should help. > > > > > > > > > > > > > > Well, when someone has both time and understanding of the tools > > > > > > > and the > > > > > > > frameworks, we can expand the automatic tests while still having > > > > > > > functional testing as people use the feature. > > > > > > > > > > > > Do you think this patch should be applied as is? Could the tests be > > > > > > handled later? > > > > > > > > > > > > Please let me know. > > > > > > > > > > Who is going to handle the tests later and when? > > > > > > > > Someone, once how to write and run the tests is documented. That's a > > > > big hurdle this private thread has shown, to me at least. > > > > > > > > -- > > > > Tom > > > > > > Hi, > > > > > > I fixed a minor issue in the patch, and sent a revised version. > > > > > > Seems that you don't have the testing quite defined yet, so I don't know > > > what I can contribute there. Maybe something can be done with the CSF > > > parser to check that the signing works correctly? > > > > Just to be clear, the testing is fully defined...this is actually the > > only entry type which doesn't have a proper test. Every other user was > > able to add a test. The goal here isn't really to check that external > > tools are working (they should have their own tests), more to check > > that Binman is doing the right thing. > > > > I will get some sort of patch out by tomorrow morning to help this process. > > > > > > > > However, I think this feature is quite benign, and it would be great to > > > get some functional testing in for this feature, as Tom said. For us, > > > this is an important feature, so we have done extensive testing > > > internally to verify that it works. > > > > I fully understand that...but of course without an automated test in > > Binman it may break at any moment, as Binman continues to be expanded. > > > > BTW, minor code-style things: U-Boot's Python code uses single quotes > > and the line limit is 80cols. > > > > Regards, > > Simon > > Hi Simon, > > > Just to be clear, the testing is fully defined...this is actually the > > only entry type which doesn't have a proper test. Every other user was > > able to add a test. The goal here isn't really to check that external > > tools are working (they should have their own tests), more to check > > that Binman is doing the right thing. > > > > I will get some sort of patch out by tomorrow morning to help this process. > > Alrighty. I'll include a test once we get this sorted out. > > > BTW, minor code-style things: U-Boot's Python code uses single quotes > > and the line limit is 80cols. > > I don't understand what you mean because I did use single quotes :) > Perhaps you were looking at the CSF template, which is not Python code?
Ah OK, I see. It would be less confusing if you used f-strings. Unfortunately Binman does not use them always and there are tons of pylint warnings about it. > > As for the column width, sorry, I was only trying to follow the pattern > in the file. I thought it was 120 cols. I will include a predecessor > patch to fix the column width to 80. Oh, OK, yes I wasn't around when the original patch was sent so didn't review it. Regards, SImon
