On Fri, 2016-03-11 at 12:05 -0300, Eduardo Lima (Etrunko) wrote: > 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) > right, my bad, I though we are using tab=4spaces I would go for your suggestion about space+'\' Thanks for the review guys, Pavel _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list