On 13 Sep 2016, at 17:22, Junio C Hamano [off-list ref] wrote:

larsxschneider@gmail.com writes:

quoted

diff --git a/contrib/long-running-filter/example.pl b/contrib/long-running-filter/example.pl

...
+sub packet_read {
+    my $buffer;
+    my $bytes_read = read STDIN, $buffer, 4;
+    if ( $bytes_read == 0 ) {
+
+        # EOF - Git stopped talking to us!
+        exit();
+...
+packet_write( "clean=true\n" );
+packet_write( "smudge=true\n" );
+packet_flush();
+
+while (1) {

These extra SP around the contents inside () pair look unfamiliar
and somewhat strange to me, but as long as they are consistently
done (and I think you are mostly being consistent), it is OK.

Ups. I forgot to run PerlTidy here. I run PerlTidy with the flag 
"-pbp" (= Perl Best Practices). This seems to add no extra SP for
functions with one parameter (e.g. `foo("bar")`) and extra SP
for functions with multiple parameter (e.g. `foo( "bar", 1 )`).
Is this still OK?

Does anyone have a "Git PerlTidy configuration"?

quoted

+#define CAP_CLEAN    (1u<<0)
+#define CAP_SMUDGE   (1u<<1)

As these are meant to be usable together, i.e. bits in a single flag
word, they are of type "unsigned int", which makes perfect sense.

Make sure your variables and fields that store them are of the same
type.  I think I saw "int' used to pass them in at least one place.

Fixed!

quoted

+static int apply_filter(const char *path, const char *src, size_t len,
+                        int fd, struct strbuf *dst, struct convert_driver *drv,
+                        const int wanted_capability)
+{
+	const char* cmd = NULL;

"const char *cmd = NULL;" of course.

Fixed!

quoted

diff --git a/unpack-trees.c b/unpack-trees.c
index 11c37fb..f6798f8 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c

@@ -10,6 +10,7 @@

#include "attr.h"
#include "split-index.h"
#include "dir.h"
+#include "convert.h"

/*
 * Error messages expected by scripts out of plumbing commands such as

Why?  The resulting file seems to compile without this addition.

Of course. That shouldn't have been part of this commit.


Thank you,
Lars