Nico Huber posted comments on this change.

View Change

Patch set 6:

Only some nits and simple things it getrevision.sh.

(8 comments)

• Commit Message:

  • Patch Set #6, Line 13: - Remove djgpp-dos target (it is not different to other x-compilations).

    I was just wondering, why I didn't nag earlier, you can keep
    the change in or remove this line...

  • Patch Set #6, Line 15: anything else from the code dump when we export the respective variable.

    Nit, line is too long.

  • Patch Set #6, Line 26: f5dd7ce1

    This sha will probably get lost. Linking to the mailing list
    archive might be better.

  • Patch Set #6, Line 28: pushing to the review server.

    Maybe also mention, that you split out the complex version
    string.

• File util/getrevision.sh:

  • Patch Set #6, Line 140:

    		local svn_base=$(git log --grep=git-svn-id -1 --format='%h')
    		if [ "$svn_base" != "" ] ; then
    			local diff_to_svn=$(git rev-list --count ${svn_base}..${r})
    			if [ "$diff_to_svn" -gt 0 ] ; then
    				r="$r-$diff_to_svn"
    			fi
    		fi
    

    Looks like we don't have git-svn-id any more. A simple `git describe
    ${r}` might do? We already have tags in git, for me it shows something
    like:

        v0.9.9-22-gfb2996c1

  • Patch Set #6, Line 168: sed 's/.*@/r/;s/[[:blank:]].*//')

    Same here, maybe just drop the --upstream command?

• File util/git-hooks/pre-push:

  • Patch Set #6, Line 46:

    	else
    		range="$remote_sha..$local_sha"
    

    Nit, actually I don't like the `else` here, it's not directly
    related to the `if` (i.e. could just move below the `fi`).

    Now I also wonder why we need this variable at all (only used
    in one place).

  • Patch Set #6, Line 55: " or \"Acked-by\" lines, not pushing." >&2

    Sounds good.

To view, visit change 19206. To unsubscribe, visit settings.