On Wed, Dec 18, 2013 at 7:18 PM, Tom Miller <jacker...@gmail.com> wrote:
> On Wed, Dec 18, 2013 at 3:47 PM, Junio C Hamano <gits...@pobox.com> wrote:
>> Tom Miller <jacker...@gmail.com> writes:
>>> In order to fix branchname DF conflicts during `fetch --prune`, the way
>>> the header is output to the screen needs to be refactored. Here is an
>>> exmaple of the output with the line in question denoted by '>':
>>>       $ git fetch --prune --dry-run upstream
>>>>      From https://github.com/git/git
>>>          a155a5f..5512ac5  maint      -> upstream/maint
>>>          d7aced9..7794a68  master     -> upstream/master
>>>          523f7c4..3e57c29  next       -> upstream/next
>>>        + 462f102...0937cdf pu         -> upstream/pu  (forced update)
>>>          e24105a..5d352bc  todo       -> upstream/todo
>>>        * [new tag]         v1.8.5.2   -> v1.8.5.2
>>>        * [new tag]         v1.8.5.2   -> v1.8.5.2
>>> pretty_url():
>>> This function when passed a transport url will anonymize the transport
>>> of the url. It will strip a trailing '/'. It will also strip a trailing
>>> '.git'. It will return the newly formated url for use. I do not believe
>>> there is a need for stripping the trailing '/' and '.git' from a url,
>>> but it was already there and I wanted to make as little changes as
>>> possible.
>> OK.  I tend to agree that stripping the trailing part is probably
>> not a good idea and we would want to remove that but that definitely
>> should be done as a separate step, or even as a separate series on
>> top of this one.
> I think that removing the trailing part will greatly reduce the complexity
> to the point were it is unnecessary to have pretty_url().  My goal with
> extracting this function is to isolate the complexity of formatting the
> url to a single spot. I am thinking along the lines of the following
> commit order:
> 1. Remove the "remove trailing part"
> 2. Add print_url()
> 3. Always print url when pruning
> 4. Reverse order of prune and fetch
>>> print_url():
>>> This function will convert a transport url to a pretty url using
>>> pretty_url(). Then it will print out the pretty url to stderr as
>>> indicated above in the example output. It uses a global variable
>>> named "gshown_url' to prevent this header for being printed twice.
>> Gaah.  What is that 'g' doing there?  Please don't do that
>> meaningless naming.
> I am not familiar with C conventions and I was trying to stay consistent.
> I saw other global variables starting with 'g' and made an assumption.
> It will use the original name in the upcoming patches.
>> I do not think the change to introduce such a global variable
>> belongs to this refactoring step.  The current caller can decide
>> itself if it called that function, and if you are going to introduce
>> new callers in later steps, they can coordinate among themselves,
>> no?
> I agree, there is no reason for introducing it in this step. Thanks for
> pointing that out.

After working on this some more and realizing there is more work to
be done on the "fetch --prune should prune before fetching" issue. Also,
seeing Jeff's response opened my eyes even more. I believe you are
correct. The "trailing parts" piece should be split off into another patch set.
I think it would make sense to add the "fetch --prune should print the header
url" to that patch set. Should I submit those patches as a separate thread
or reply to this thread with just those patches?

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

Reply via email to