Re: [PATCH 3/5] setup_revisions: remember whether a ref was positive or not

Sverre Rabbelier [off-list ref] writes:

quoted

But the point is that this codepath does not have a faintest clue whether
the "obj" parameter is something the end user actively asked for (which
might have been marked as uninteresting for other reasons, namely, because
it is reachable from other negative refs). So passing unconditional 0 is
just as bad.

Doesn't passing 0 indicate that we _did not_ receive any explicit user
input on this ref, which is exactly what we want to record? If the
user passed us any explicit input on the ref, we record it at the
other call-sites, here, the user told us nothing, so we record exactly
that, nothing.

Hmm, which means you have a way to say "explicitly affirmative" vs "no
information", but no way to say "explicitly negative", for example, and
the worse part is that it is unclear if the approach the patch takes is
extensible enough to allow that in the future. That is the kind of "myopic
hack" attitude I did not particularly like in this patch.

"The next person who needs more generic framework can rip out what this
patch does and the work required is the same amount" is not a convincing
argument---it would mean you are burdening that other person with an extra
work to _redo_ what this series does properly, and it is not likely to be
of help for that person after your interest in this codepath has long
waned.

quoted

As I said, I am not married to the verbose struct/union representation
(the only reason I showed that verbosity was because it allowed me to do
away without having to enumerate all the syntax sugars we already
support); if your "flags" can express the same thing (it may needs to
become a bitfield with enough width, but I highly suspect that you would
also need at least a component that says "this is the string the user gave
us --- the user said 'master', not 'master^0', for example) and is a lot
more compact, that is definitely we want to go with.

Don't we already store that in the name field?

Please remind yourself why then it is not sufficient for your patch to
read from the name field please?

After all, wasn't the issue that "master^0..master" yields an empty set
but you somehow wanted to know that the RHS of that dotdot was given as a
positive ref?

Heya,

On Tue, Aug 9, 2011 at 00:24, Junio C Hamano [off-list ref] wrote:

Hmm, which means you have a way to say "explicitly affirmative" vs "no
information", but no way to say "explicitly negative", for example, and
the worse part is that it is unclear if the approach the patch takes is
extensible enough to allow that in the future. That is the kind of "myopic
hack" attitude I did not particularly like in this patch.

Do we have a way to say explicitly negative on the commandline? Is
there a way to say "I don't want you to decorate this commit as
anything at all"?

"The next person who needs more generic framework can rip out what this
patch does and the work required is the same amount" is not a convincing
argument---it would mean you are burdening that other person with an extra
work to _redo_ what this series does properly, and it is not likely to be
of help for that person after your interest in this codepath has long
waned.

Be fair. How is "2 files changed, 8 insertions(+), 6 deletions(-)"
going to make it any harder at all for someone who is going to be
doing that huge patch series you described earlier?

quoted

Don't we already store that in the name field?

Please remind yourself why then it is not sufficient for your patch to
read from the name field please?

Sure, we could do it. But it would be duplicating all the effort
already being done in rev-parse!

After all, wasn't the issue that "master^0..master" yields an empty set
but you somehow wanted to know that the RHS of that dotdot was given as a
positive ref?

The issue was that if I push "master" to origin but I already pushed a
"next" which points at the same commit nothing happens because
git-fast-export doesn't know to emit the "reset master :13878" line.

-- 
Cheers,

Sverre Rabbelier