Jeff King <peff@xxxxxxxx> writes: >> > + if (rs->fetch == REFSPEC_FETCH) { >> >> Do you think it'd be worth handling rs->fetch in a switch/case block? At >> least that would allow us to catch unknown values more easily, though it >> seems unlikely we'd ever add any :-). > > ...this whole thing is badly named. It is called "fetch", but the only > two values are true/false. But for some reason we named them > REFSPEC_FETCH and REFSPEC_PUSH. Surely it should be "type" or > "operation" or something if we were going to use an enum and switch? Sorry, I suspect that it is my fault. I've never been good at naming, and if the allowed values for this member are FETCH and PUSH, then the member itself shouldn't be called fetch or push. Perhaps 'direction'. But such a name improvement is clearly outside the scope of this fix. Thanks.