Hello,

Am Samstag, 28. November 2015 schrieb John Johansen:
> v3
> 
> change conflicting/unknown option warning message slightly
> output error string on failure
> add binutils dir
> add manpage
> add makefile
> add pot file

> === added file 'binutils/Makefile'
> --- binutils/Makefile 1970-01-01 00:00:00 +0000
> +++ binutils/Makefile 2015-11-28 18:18:25 +0000

It looks like you copied large parts of parser/Makefile.
Would it make sense to split those common parts off to a separate file, 
like common/Make-c.rules?

(That's nothing that should stop you from adding aa-enabled, so feel 
free to do that as a follow-up patch.)

BTW: It seems you never commited the parser/Makefile cleanup patch series 
you sent a while ago. Is there a special reason, or did you just forget 
it? (Also, does binutils/Makefile need some similar cleanups, or are they 
already integrated?)

> === added file 'binutils/aa-enabled.c'
> --- binutils/aa-enabled.c       1970-01-01 00:00:00 +0000
> +++ binutils/aa-enabled.c       2015-11-28 17:34:45 +0000
...
> +#ifndef PACKAGE
> +#define PACKAGE ""
> +#define LOCALEDIR ""
> +#endif

Now that we have a nice Makefile, is this still needed?

> === added file 'binutils/po/aa-enabled.pot'
> --- binutils/po/aa-enabled.pot        1970-01-01 00:00:00 +0000
> +++ binutils/po/aa-enabled.pot        2015-11-28 18:23:11 +0000
> @@ -0,0 +1,67 @@
> +# SOME DESCRIPTIVE TITLE.
> +# Copyright (C) YEAR Canonical Ltd
> +# This file is distributed under the same license as the PACKAGE
> package. +
> # FIRST AUTHOR <EMAIL@ADDRESS>, YEAR.

That's a very informative copyright header, especially the uppercase 
parts ;-)


That said: The patch looks good to me (with the questions answered or 
addressed), but I'll leave acking it for someone who understands C 
better.


Regards,

Christian Boltz
-- 
Diese Signatur ist vorübergehend nicht erreichbar.
Versuchen Sie es später noch einmal oder hinterlassen
Sie eine Nachricht vor dem Signaturtrenner. Piep.


-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to