On Sat, 22 Mar 2008, Junio C Hamano wrote:
> Daniel Barkalow <barkalow@xxxxxxxxxxxx> writes:
>
> > On Fri, 21 Mar 2008, Junio C Hamano wrote:
> > ...
> >> Do you mean you want the callers of this internal implementation to also
> >> loop over the input set of refs? I think that would be more complex code
> >> but I do not see much gain by doing so.
> >
> > I think it's more breadth but less depth. It would make the internal
> > implementation not depend on fetch, and put the checks that only apply to
> > fetch out of the push code path and vice versa.
> >
> > Or just have a section
> >
> > if (fetch) {
> > // checks for fetch LHS
> > // checks for fetch RHS
> > } else {
> > // checks for push LHS
> > // checks for push RHS
> > }
> >
> > The body of the condition is only four lines, after all.
>
> There are two commits on jc/refspec-fix branch merged to 'pu'. The
> earlier one is my version, and the one on top is based on the above
> suggestion. I do not know which one is clearer, more readable and
> maintainable.
I like the second one quite a bit. If nothing else, it's got, as the
comments, the best documentation of refspecs I've seen, and then it's got
code which clearly implements those rules. I think the explicitness is
worth the extra lines.
Thanks for taking care of this; as a result of starting a new job, I
haven't had much in the way of coding attention for other stuff the past
couple of weeks.
-Daniel
*This .sig left intentionally blank*
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
