Re: [PATCH v2 00/11] sha1_name: improvements

2013-05-08 Thread Ramkumar Ramachandra 
Junio C Hamano wrote:
 Will queue.  Thanks.

Nit: you might want to add my s-o-b on patches 73027d and b018e8 queued in pu.
--
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 v2 00/11] sha1_name: improvements

2013-05-07 Thread Felipe Contreras 
Hi,

While trying to add support for the @ shortcut lots of cleanups arised. Here
they are in a single series.

Felipe Contreras (7):
  tests: at-combinations: simplify setup
  tests: at-combinations: check ref names directly
  tests: at-combinations: improve nonsense()
  sha1_name: remove no-op
  sha1_name: remove unnecessary braces
  sha1_name: avoid Yoda conditions
  sha1_name: reorganize get_sha1_basic()

Ramkumar Ramachandra (4):
  tests: at-combinations: increase coverage
  tests: at-combinations: @{N} versus HEAD@{N}
  sha1_name: don't waste cycles in the @-parsing loop
  sha1_name: check @{-N} errors sooner

 sha1_name.c| 42 +++---
 t/t1508-at-combinations.sh | 56 +-
 2 files changed, 64 insertions(+), 34 deletions(-)

-- 
1.8.3.rc0.401.g45bba44

--
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

Re: [PATCH v2 00/11] sha1_name: improvements

2013-05-07 Thread Felipe Contreras 
On Tue, May 7, 2013 at 4:55 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 While trying to add support for the @ shortcut lots of cleanups arised. Here
 they are in a single series.

 Felipe Contreras (7):
   tests: at-combinations: simplify setup
   tests: at-combinations: check ref names directly
   tests: at-combinations: improve nonsense()
   sha1_name: remove no-op
   sha1_name: remove unnecessary braces
   sha1_name: avoid Yoda conditions
   sha1_name: reorganize get_sha1_basic()

 Ramkumar Ramachandra (4):
   tests: at-combinations: increase coverage
   tests: at-combinations: @{N} versus HEAD@{N}
   sha1_name: don't waste cycles in the @-parsing loop
   sha1_name: check @{-N} errors sooner

When merging this series to the @ shortcut one, there will be
conflicts, this is how I propose fixing them:

return len; /* syntax Ok, not enough switches */
-   if (0  len  len == namelen)
+   if (len  0  len == namelen)
return len; /* consumed all */
-   else if (0  len)
...
++  else if (len  0)
 +  return reinterpret(name, namelen, len, buf);

- check @ new-two
- check @@{u} upstream-two
...
++check @ ref refs/heads/new-branch
++check @@{u} ref refs/heads/upstream-branch

If that creates some kind of problem I would rather throw away this
series rather than the other one.

Cheers.

-- 
Felipe Contreras
--
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

Re: [PATCH v2 00/11] sha1_name: improvements

2013-05-07 Thread Junio C Hamano 
Felipe Contreras felipe.contre...@gmail.com writes:

 When merging this series to the @ shortcut one, there will be
 conflicts, this is how I propose fixing them:

 return len; /* syntax Ok, not enough switches */
 -   if (0  len  len == namelen)
 +   if (len  0  len == namelen)
 return len; /* consumed all */
 -   else if (0  len)
 ...
 ++  else if (len  0)
  +  return reinterpret(name, namelen, len, buf);

 - check @ new-two
 - check @@{u} upstream-two
 ...
 ++check @ ref refs/heads/new-branch
 ++check @@{u} ref refs/heads/upstream-branch

The resolution for the tests wrapper that acquired an extra
parameter matches what I did locally.  Thanks for a merge sanity
check.

I didn't see any conflicts on the sha1_name.c side, but I applied
the Yoda thing slightly differently to result in a slightly more
streamlined codeflow:

if (!len) {
return len;
} else if (len  0) {
if (len == namelen)
return len;
else
return reinterpret(...);
}

which I think shows the choices better.

Although I haven't looked at the largest one (10/11) carefully,
everything else looked quite straightforward and readable.

I am not very happy about how $n parameters are quoted in t1508,
but that suboptimal quoting were there before this series, and I'd
consider it outside of the scope for now.

Will queue.  Thanks.


--
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