On Mon, Feb 19, 2024 at 03:01:29PM +0100, Martin Kletzander wrote:
> On Mon, Feb 19, 2024 at 04:47:52AM -0800, Andrea Bolognani wrote:
> > On Mon, Feb 19, 2024 at 10:35:14AM +0100, Martin Kletzander wrote:
> > > +if missing_optional_programs.length() > 0
> > > + misc_summary += {'Some programs are missing, not all tests will be executed':
> > > + missing_optional_programs}
> > > +endif
> >
> > I like it, but I'm going to suggest a slightly tweaked
> > implementation.
> >
> > With the diff below squashed in, the output will turn into
> >
> > Optional programs
> > Missing : black (some tests will be skipped!)
> >
> > which is less busy and more readable IMO. I think it's more likely to
> > catch the user's eye compared to being yet another line at the bottom
> > of the Miscellaneous section.
>
> I liked that the first column got wider, but to be honest it catches
> your eye only if you are used to the output.
>
> And I like your version better. If you have it squashed locally, then
> feel free to push that.
I think it's probably easier if you just squash it in yourself. You
can also add my
Reviewed-by: Andrea Bolognani <abologna@xxxxxxxxxx>
to both patches when you do that.
Maybe give Michal a chance to object to the change before pushing,
just in case it would make him retract his R-b.
> > + summary(test_programs_summary, section: 'Optional programs', bool_yn: true)
>
> the bool_yn doesn't do anything here, but that's fine
Yeah, I copied it from the existing invocations but you're right that
it's unnecessary here. By all means drop it.
--
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
