diff mbox series

[FFmpeg-devel,1/4] tests/checkasm: cosmetics, one object per line in Makefile

Message ID 20240607140543.130761-1-ramiro.polla@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/4] tests/checkasm: cosmetics, one object per line in Makefile | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Ramiro Polla June 7, 2024, 2:05 p.m. UTC 
---
 tests/checkasm/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Andreas Rheinhardt June 7, 2024, 6:45 p.m. UTC | #1 
Ramiro Polla:
> ---
>  tests/checkasm/Makefile | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile
> index 6eb94d10d5..3ce152e818 100644
> --- a/tests/checkasm/Makefile
> +++ b/tests/checkasm/Makefile
> @@ -63,7 +63,9 @@ AVFILTEROBJS-$(CONFIG_SOBEL_FILTER)      += vf_convolution.o
>  CHECKASMOBJS-$(CONFIG_AVFILTER) += $(AVFILTEROBJS-yes)
>  
>  # swscale tests
> -SWSCALEOBJS                             += sw_gbrp.o sw_rgb.o sw_scale.o
> +SWSCALEOBJS                             += sw_gbrp.o
> +SWSCALEOBJS                             += sw_rgb.o
> +SWSCALEOBJS                             += sw_scale.o
>  
>  CHECKASMOBJS-$(CONFIG_SWSCALE)  += $(SWSCALEOBJS)
>  

We use the multiple-objects in a line style in all Makefiles.

- Andreas
Ramiro Polla June 7, 2024, 7:09 p.m. UTC | #2 
On Fri, Jun 7, 2024 at 8:46 PM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> Ramiro Polla:
> > ---
> >  tests/checkasm/Makefile | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile
> > index 6eb94d10d5..3ce152e818 100644
> > --- a/tests/checkasm/Makefile
> > +++ b/tests/checkasm/Makefile
> > @@ -63,7 +63,9 @@ AVFILTEROBJS-$(CONFIG_SOBEL_FILTER)      += vf_convolution.o
> >  CHECKASMOBJS-$(CONFIG_AVFILTER) += $(AVFILTEROBJS-yes)
> >
> >  # swscale tests
> > -SWSCALEOBJS                             += sw_gbrp.o sw_rgb.o sw_scale.o
> > +SWSCALEOBJS                             += sw_gbrp.o
> > +SWSCALEOBJS                             += sw_rgb.o
> > +SWSCALEOBJS                             += sw_scale.o
> >
> >  CHECKASMOBJS-$(CONFIG_SWSCALE)  += $(SWSCALEOBJS)
> >
>
> We use the multiple-objects in a line style in all Makefiles.

Then we should change the following:
libswscale/arm/Makefile (NEON_OBJS)
tests/checkasm/Makefile (AVUTILOBJS)
libavfilter/dnn/Makefile (OBJS-$(CONFIG_DNN))

New patch attached.
Andreas Rheinhardt June 7, 2024, 7:12 p.m. UTC | #3 
Ramiro Polla:
>  # swscale tests
> -SWSCALEOBJS                             += sw_gbrp.o sw_rgb.o sw_scale.o
> +SWSCALEOBJS                             += sw_gbrp.o \
> +                                           sw_rgb.o \
> +                                           sw_scale.o \
>  
>  CHECKASMOBJS-$(CONFIG_SWSCALE)  += $(SWSCALEOBJS)

We typically only use a new line of the old line is full.

- Andreas
Ramiro Polla June 7, 2024, 7:47 p.m. UTC | #4 
On Fri, Jun 7, 2024 at 9:27 PM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> Ramiro Polla:
> >  # swscale tests
> > -SWSCALEOBJS                             += sw_gbrp.o sw_rgb.o sw_scale.o
> > +SWSCALEOBJS                             += sw_gbrp.o \
> > +                                           sw_rgb.o \
> > +                                           sw_scale.o \
> >
> >  CHECKASMOBJS-$(CONFIG_SWSCALE)  += $(SWSCALEOBJS)
>
> We typically only use a new line of the old line is full.

There's currently a mix of everything in the Makefiles. One object per
line, multiple objects per line, mix of one or multiple objects per
line in the same statement, aligned and unaligned += between lines,
aligned and unaligned \ at the end of the lines, some have \ at the
last line, some don't...

I personally prefer += one object per line and no \ at the end of the
line everywhere. It makes the code look consistent and the patches are
cleaner and easier to understand. But I don't maintain this, so I have
no strong opinion in this case.

This patch was meant to simplify the next commit (checkasm: add tests
for {lum,chr}ConvertRange), but I can drop it if you prefer.
diff mbox series

Patch

diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile
index 6eb94d10d5..3ce152e818 100644
--- a/tests/checkasm/Makefile
+++ b/tests/checkasm/Makefile
@@ -63,7 +63,9 @@  AVFILTEROBJS-$(CONFIG_SOBEL_FILTER)      += vf_convolution.o
 CHECKASMOBJS-$(CONFIG_AVFILTER) += $(AVFILTEROBJS-yes)
 
 # swscale tests
-SWSCALEOBJS                             += sw_gbrp.o sw_rgb.o sw_scale.o
+SWSCALEOBJS                             += sw_gbrp.o
+SWSCALEOBJS                             += sw_rgb.o
+SWSCALEOBJS                             += sw_scale.o
 
 CHECKASMOBJS-$(CONFIG_SWSCALE)  += $(SWSCALEOBJS)