Re: [PATCH 1/3] builtin/fetch.c: Add pretty_url() and print_url()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Tom Miller <jackerran@xxxxxxxxx> 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.

> 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 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?

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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]