Code review comment for lp:~allenap/juju-core/makefile-stuff

Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2013-08-20 21:52, Gavin Panella wrote:
> Gavin Panella has proposed merging
> lp:~allenap/juju-core/makefile-stuff into lp:juju-core.
>
> Requested reviews: juju hackers (juju)
>
> For more details, see:
> https://code.launchpad.net/~allenap/juju-core/makefile-stuff/+merge/181113
>
> Improve build tooling
>
> Some build targets - build, check and install - can only run when
> the tree is in the right place on GOPATH. The Makefile now guards
> this and provides useful errors.
>
> The system package dependencies are now more readably listed.
>
> The ~juju PPAs are not added on Saucy; they do not work there.
>
> A new clean target removes test executables. These are also
> ignored.
>

> Editor artifacts - tag files, .emacs.desktop* - are no longer
> ignored. These belong in a developer's local ~/.bazaar/ignore
> file, not in a project's.
>

While I sort of agree, I don't find it harmful and it means things
"just work" for people who haven't finished configuring them. Why do
you feel they must be removed? (IOW, while I slightly agree the net
benefit of having them in there outweighs the tiny downside of having
a slightly larger ignore file, but maybe I'm missing something.)

The rest is fine to me though:
+clean:
+ find . -name '*.test' -print0 | xargs -r0 $(RM) -v

Should probably run 'go clean' somehow. Or at least delete 'go build'
output for various cmd programs.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlIUmRAACgkQJdeBCYSNAAMriQCgvdO5ZnYzHHRzq5uCw+nVl2JB
dYAAoMhNEUCJ+wbLgiCEXAusGw3sj1yv
=D3av
-----END PGP SIGNATURE-----

« Back to merge proposal