<p dir="ltr"><br>
On Jan 7, 2014 5:52 PM, "Olof Johansson" <<a href="mailto:olof.johansson@axis.com">olof.johansson@axis.com</a>> wrote:<br>
><br>
> On 14-01-07 16:02 +0100, Richard Purdie wrote:<br>
> > On Tue, 2014-01-07 at 15:46 +0100, Olof Johansson wrote:<br>
> > > I agree with your reasoning with regards to not needing a<br>
> > > fallback for Andrei's patch, but my patch tries to first resolve<br>
> > > refs/tags/<tag> (fully qualified) and then falls back to try<br>
> > > refs/heads/<branch>. The reason for this is that ls-remote will<br>
> > > return things like refs/heads/lalala/<tag> if you only do<br>
> > > ls-remote <tag>.<br>
> ><br>
> > Can we somehow anchor the expression instead e.g. refs/*/<xxx>^{}<br>
><br>
> Sure, we can do ls-remote without any arguments and parse the<br>
> output. But this won't make the patch easier in any way. One less<br>
> fork though.<br>
><br>
> > > My attempt was also to break out the tasks into simple,<br>
> > > atomic(-ish) functions that would later be easier to write unit<br>
> > > tests for. You complained on IRC that changes to the fetcher<br>
> > > usually comes back to bite you. I believe increased unit test<br>
> > > coverage on low level functions, as a compliment to the existing<br>
> > > more high level suite, can be beneficial in this regard.<br>
> ><br>
> > I believe that my complaint could also be addressed by ensuring the high<br>
> > level tests had more coverage of the variety of supported use cases. In<br>
> > this case a range of tags of differing types, branches and hanging<br>
> > commits would have helped ensure the changes to the fetcher didn't break<br>
> > things.<br>
><br>
> Yes, that probably possible. *I* find low level unit tests easier<br>
> to work with, and I wanted to do what I could to alleviate the<br>
> situation; making sure that (at least) our use cases are covered<br>
> by automated tests.<br>
><br>
> > I'm less keen on forcing an atom like set of functions on the fetcher<br>
> > just for the purposes of the test suite. Sorry I was unclear on that, I<br>
> > guess different people have different understandings of what increased<br>
> > coverage would mean.<br>
><br>
> I find it easier to work in functions with limited scopes, with<br>
> clear purposes --- with or without unittests (unittests being<br>
> preferable, obviously). But then again, I'm not the maintainer,<br>
> so your preferences are more important :-).<br>
><br>
> > > Also, if you are holding things off, I would really appreciate<br>
> > > you letting us know so that we can make appropriate workarounds<br>
> > > if needed (as in this case, where we have had to supsend testing<br>
> > > on master :-().<br>
> ><br>
> > Well, it wasn't a conscious decision so much as the holidays came along<br>
> > and I ran out of time to look at it. I'm trying to get this resolved now<br>
> > since obviously various people are hitting the problem cases.<br>
><br>
> I really appreciate the hard work you are doing :-).</p>
<p dir="ltr">Me too :-)</p>
<p dir="ltr">I agree that we really need to fix this as we are breaking layers out in the wild with this issue.</p>
<p dir="ltr">@olof sorry for duplicate email.</p>
<p dir="ltr">ag<br>
</p>