[Spice-devel] [PATCH] tests: add explanation for test_qxl_pasring_SOURCES

[Jonathon Jongsma]

On Wed, 2016-01-20 at 11:18 -0500, Frediano Ziglio wrote:
> > 
> > On Tue, 2016-01-19 at 05:42 -0500, Frediano Ziglio wrote:
> > > > Subject: [PATCH] tests: add explanation for test_qxl_pasring_SOURCES
> > > > 
> > > 
> > > Typo on subject. Already fixed
> > > 
> > > > Instead of using libserver.a to include necessary functions we include
> > > > single files to check for dependencies.
> > 
> > what do you mean exactly by "to check for dependencies"
> > 
> 
> memslot.c and red-parse-qxl.c are kind of base modules.
> There is no reason they depends on something else on spice-server.
> The link command make sure this is and will be true (you'll get a linker
> error).
> However is hard to spot from the Makefile so I added a comment.
> I think the importance of this is quite an opinion.

OK, fair point. I'm OK with that. Can you change the commit log to something
like: 

"to ensure that they don't depend on any other files"

Acked-by: Jonathon Jongsma <jjongsma at redhat.com>



> 
> Frediano
> 
> 
> > 
> > > > 
> > > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > > ---
> > > >  server/tests/Makefile.am | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > > 
> > > > diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am
> > > > index 8caff04..892a60b 100644
> > > > --- a/server/tests/Makefile.am
> > > > +++ b/server/tests/Makefile.am
> > > > @@ -91,6 +91,8 @@ libstat_test3_a_CPPFLAGS = $(AM_CPPFLAGS)
> > > > -DTEST_COMPRESS_STAT=1 -DTEST_RED_WORK
> > > >  libstat_test4_a_SOURCES = stat-test.c
> > > >  libstat_test4_a_CPPFLAGS = $(AM_CPPFLAGS) -DTEST_COMPRESS_STAT=1
> > > >  -DTEST_RED_WORKER_STAT=1 -DTEST_NAME=stat_test4
> > > >  
> > > > +# Here we include only needed files to make sure code does not depend
> > > > +# on extra files.
> > 
> > OK, but why is this important?
> > 
> > > >  test_qxl_parsing_SOURCES =           \
> > > >  	test-qxl-parsing.c      \
> > > >  	../red-parse-qxl.c      \
> > > > --
> > > > 2.4.3
> > > > 
> > > > 
> > > _______________________________________________
> > > Spice-devel mailing list
> > > Spice-devel at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 
> > Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>
> >