[virt-tools-list] [PATCH virt-viewer 2/2] Move tests under /tests directory

Eduardo Lima (Etrunko) etrunko at redhat.com
Fri Mar 11 15:05:26 UTC 2016


On 03/11/2016 11:44 AM, Fabiano Fidêncio wrote:
[snip]
>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>> new file mode 100644
>> index 0000000..470733b
>> --- /dev/null
>> +++ b/tests/Makefile.am
>> @@ -0,0 +1,30 @@
>> +NULL =
>> +
>> +AM_CPPFLAGS =                                                                  \
>> +       -DLOCALE_DIR=\""$(datadir)/locale"\"            \
>> +       -I$(top_srcdir)/src/                                            \
>> +       -I$(top_srcdir)/tests/                                          \
>> +       $(GLIB2_CFLAGS)                                                         \
>> +       $(GTK_CFLAGS)                                                           \
>> +       $(LIBXML2_CFLAGS)                                                       \
>> +       $(WARN_CFLAGS)                                                          \
>> +       $(NULL)
> 
> Pavel, I know you basically copied & pasted the old code here. But do
> we need {GTK,LIBXML2}_CFLAGS here? Our tests are not using none of
> those if I'm not mistaken.
> 
> I would accept a first patch removing these unneeded CPPFLAGS/LIBS.
> 
> Also, can we have the "\" aligned?
> 
>> +
>> +LDADD=                                                                                 \
>> +       $(top_builddir)/src/libvirt-viewer-util.la      \
>> +       $(GLIB2_LIBS)                                                           \
>> +       $(GTK_LIBS)                                                                     \
>> +       $(LIBXML2_LIBS)                                                         \
>> +       $(NULL)
>> +
> 
> Same here.
> 

The problem with having backslash symbols aligned is that each one's
preferred editor might have different setups. Makefiles use to require
tab indentation (not sure if still a hard requirement), so different
tabstops may result in different alignment.

My suggestion is to just use a single space after the file/variable and
then the backslash symbol, while the begin of each line is still tab
indented. For instance:

+LDADD= \
+       $(top_builddir)/src/libvirt-viewer-util.la \
+       $(GLIB2_LIBS) \
+       $(GTK_LIBS) \
+       $(LIBXML2_LIBS) \
+       $(NULL)

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko at redhat.com




More information about the virt-tools-list mailing list