Re: [PATCH 08/23] Extract function should_expire_reflog_ent()

[Stefan Beller]

On Fri, Dec 05, 2014 at 12:08:20AM +0100, Michael Haggerty wrote:
 Extracted from expire_reflog_ent() a function that is solely
 responsible for deciding whether a reflog entry should be expired. By
 separating this business logic from the mechanics of actually
 expiring entries, we are working towards the goal of encapsulating
 reflog expiry within the refs API, with policy decided by a callback
 function passed to it by its caller.
 
 Signed-off-by: Michael Haggerty mhag...@alum.mit.edu

Reviewed-by: Stefan Beller sbel...@google.com

The comments below are just thoughts, which don't need to be
included into this commit.


 + if (should_expire_reflog_ent(osha1, nsha1, email, timestamp, tz,
 +  message, cb_data)) {
 + if (!cb-newlog)
 + printf(would prune %s, message);
 + else if (cb-cmd-verbose)
 + printf(prune %s, message);

While this commit is just shoveling code around, we don't want to introduce
changes here. So a question for a possible later follow up:
git reflog is listed as an ancillary manipulator, which still is porcelain.
So we maybe want to translate [would] prune?


 + char sign = (tz  0) ? '-' : '+';
 + int zone = (tz  0) ? (-tz) : tz;
 + fprintf(cb-newlog, %s %s %s %lu %c%04d\t%s,
 + sha1_to_hex(osha1), sha1_to_hex(nsha1),
 + email, timestamp, sign, zone,
 + message);

This is fine for just moving code around and reviewing.
I send a patch on top of this one to remove the manual calculation of the 
sign and zone and let the fprintf function figure it out.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 08/23] Extract function should_expire_reflog_ent()

[Michael Haggerty]

Extracted from expire_reflog_ent() a function that is solely
responsible for deciding whether a reflog entry should be expired. By
separating this business logic from the mechanics of actually
expiring entries, we are working towards the goal of encapsulating
reflog expiry within the refs API, with policy decided by a callback
function passed to it by its caller.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 builtin/reflog.c | 70 +---
 1 file changed, 42 insertions(+), 28 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index d344d45..7bc6e0f 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -288,51 +288,65 @@ static int unreachable(struct expire_reflog_cb *cb, 
struct commit *commit, unsig
return !(commit-object.flags  REACHABLE);
 }
 
-static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
-   const char *email, unsigned long timestamp, int tz,
-   const char *message, void *cb_data)
+/*
+ * Return true iff the specified reflog entry should be expired.
+ */
+static int should_expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+   const char *email, unsigned long timestamp, 
int tz,
+   const char *message, void *cb_data)
 {
struct expire_reflog_cb *cb = cb_data;
struct commit *old, *new;
 
if (timestamp  cb-cmd-expire_total)
-   goto prune;
-
-   if (cb-cmd-rewrite)
-   osha1 = cb-last_kept_sha1;
+   return 1;
 
old = new = NULL;
if (cb-cmd-stalefix 
(!keep_entry(old, osha1) || !keep_entry(new, nsha1)))
-   goto prune;
+   return 1;
 
if (timestamp  cb-cmd-expire_unreachable) {
if (cb-unreachable_expire_kind == UE_ALWAYS)
-   goto prune;
+   return 1;
if (unreachable(cb, old, osha1) || unreachable(cb, new, nsha1))
-   goto prune;
+   return 1;
}
 
if (cb-cmd-recno  --(cb-cmd-recno) == 0)
-   goto prune;
-
-   if (cb-newlog) {
-   char sign = (tz  0) ? '-' : '+';
-   int zone = (tz  0) ? (-tz) : tz;
-   fprintf(cb-newlog, %s %s %s %lu %c%04d\t%s,
-   sha1_to_hex(osha1), sha1_to_hex(nsha1),
-   email, timestamp, sign, zone,
-   message);
-   hashcpy(cb-last_kept_sha1, nsha1);
-   }
-   if (cb-cmd-verbose)
-   printf(keep %s, message);
+   return 1;
+
return 0;
- prune:
-   if (!cb-newlog)
-   printf(would prune %s, message);
-   else if (cb-cmd-verbose)
-   printf(prune %s, message);
+}
+
+static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+   const char *email, unsigned long timestamp, int tz,
+   const char *message, void *cb_data)
+{
+   struct expire_reflog_cb *cb = cb_data;
+
+   if (cb-cmd-rewrite)
+   osha1 = cb-last_kept_sha1;
+
+   if (should_expire_reflog_ent(osha1, nsha1, email, timestamp, tz,
+message, cb_data)) {
+   if (!cb-newlog)
+   printf(would prune %s, message);
+   else if (cb-cmd-verbose)
+   printf(prune %s, message);
+   } else {
+   if (cb-newlog) {
+   char sign = (tz  0) ? '-' : '+';
+   int zone = (tz  0) ? (-tz) : tz;
+   fprintf(cb-newlog, %s %s %s %lu %c%04d\t%s,
+   sha1_to_hex(osha1), sha1_to_hex(nsha1),
+   email, timestamp, sign, zone,
+   message);
+   hashcpy(cb-last_kept_sha1, nsha1);
+   }
+   if (cb-cmd-verbose)
+   printf(keep %s, message);
+   }
return 0;
 }
 
-- 
2.1.3

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html