[Freeipa-devel] [PATCH 0069] Add 'review' target for make

[Lukas Slebodnik]

On (10/12/15 18:04), Petr Spacek wrote:
>On 9.12.2015 15:30, Petr Spacek wrote:
>> Hello,
>> 
>> this patch automates some of sanity checks proposed by Petr Vobornik.
>> 
>> 'make review' should be used in root of clean Git tree which has patches under
>> review applied.
>>
+1 for automatisation
-1 for name.

Your script does not review[1] anything. Code review(peer review) is a job
for humans. What do you think about alternative
names: "review-helper", "sanity-check" ...

>> Magic in review.sh attempts to detect nearest remote branch which can be used
>> as diff base for review. Please see review.sh for further details.
>
>And here is the patch! :-)
>
>-- 
>Petr^2 Spacek

[1] https://en.wikipedia.org/wiki/Code_review

>From 52393bf2bb0ad74dbe37e496b1fd41a6ab22bd90 Mon Sep 17 00:00:00 2001
>From: Petr Spacek <pspacek at redhat.com>
>Date: Tue, 8 Dec 2015 12:06:33 +0100
>Subject: [PATCH] Add 'review' target for make. It automates following tasks:
>
>- check if ACI.txt and API.txt are up-to-date
>- check if VERSION was changed if API was changed
>- pep8 --diff does not produce new errors when ran on diff from origin/branch
>- make lint does not produce errors
>---
> Makefile  |  4 ++++
> review.sh | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 68 insertions(+)
> create mode 100755 review.sh
>
>diff --git a/Makefile b/Makefile
>index d2c37f1597a011af2bd9ef1a4f7ce87ac59620a3..b85b3a5d6362150dcebf16745414c3d416c9547f 100644
>--- a/Makefile
>+++ b/Makefile
>@@ -79,6 +79,10 @@ check: bootstrap-autogen server tests
> 		(cd $$subdir && $(MAKE) check) || exit 1; \
> 	done
> 
>+# works only in Git tree
>+review: version-update
>+	./review.sh
>+
> bootstrap-autogen: version-update client-autogen
> 	@echo "Building IPA $(IPA_VERSION)"
> 	cd asn1; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi
>diff --git a/review.sh b/review.sh
>new file mode 100755
>index 0000000000000000000000000000000000000000..3a5114fbe88d3aba8e926e049500f5ac5f41a83e
>--- /dev/null
>+++ b/review.sh
>@@ -0,0 +1,64 @@
>+#!/bin/bash
>+is_tree_clean() {
>+	test "$(git status --porcelain "$@")" == ""
>+	return $?
>+}
>+
>+log_error() {
>+	echo "ERROR: ${1}, continuing ..."
>+	errs=(${errs[@]} "ERROR: ${1}\n")
>+}
>+
>+set -o errexit -o nounset #-o xtrace
>+
>+LOGFILE="$(mktemp --suff=.log)"
>+exec > >(tee -i ${LOGFILE})
>+exec 2>&1
>+
>+echo -n "make lint is running ... "
>+make --silent lint || log_error "make lint failed"
>+
>+# Go backwards in history until you find a remote branch from which current
>+# branch was created. This will be used as base for git diff.
>+PATCHCNT=0
>+BASEBRANCH=""
If base branch means the remote branch which was used for cloning the cerrent
git HEAD than you can simplify it a littl bit.
git rev-parse --symbolic-full-name @{upstream}

>+CURRBRANCH="$(git branch --remote --contains)"
>+while [ "${BASEBRANCH}" == "" ]
>+do
>+	BASEBRANCH="$(git branch --remote --contains "HEAD~${PATCHCNT}" | grep -v "^. ${CURRBRANCH}$" || :)"
>+	PATCHCNT="$(expr "${PATCHCNT}" + 1)"
Then for patch count you can use
PATCHCNT=$(git log --oneline $BASEBRANCH..HEAD | wc -l)

LS