Re: [PATCH v2 1/4] Refactoring: remove duplicated code from builtin-send-pack.c and transport.c

[Tay Ray Chuan <rctay89@gmail.com>]

Hi,

On Mon, 15 Feb 2010 23:26:47 +0000
Michael Lukashov <michael.lukashov@gmail.com> wrote:

> The following functions are (almost) identical:
> 
>   verify_remote_names
>   update_tracking_ref
>   print_ref_status
>   status_abbrev
>   print_ok_ref_status
>   print_one_push_status
>   refs_pushed
>   print_push_status
> 
> Move common versions of these functions to transport.c and rename them,
> as suggested by Jeff King and Junio C Hamano

this is misleading. This list should the 4 functions added to
transport.h. Some of the functions have been removed entirely from
builtin-send-pack.c and aren't renamed at all (eg. print_ref_status,
print_one_push_status).

For these, you could put them in another list, and say that "they have
been removed entirely and will not be made public, since they are only
used internally by print_push_status()."

> diff --git a/transport.c b/transport.c
> index 3846aac..0924288 100644
> --- a/transport.c
> +++ b/transport.c
> [snip]
> @@ -609,9 +609,8 @@ static void update_tracking_ref(struct remote *remote, struct ref *ref, int verb
>  	}
>  }
>  
> -#define SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
> -
> -static void print_ref_status(char flag, const char *summary, struct ref *to, struct ref *from, const char *msg, int porcelain)
> +static void print_ref_status(char flag, const char *summary, struct ref *to,
> +		  struct ref *from, const char *msg, int porcelain)
>  {
>  	if (porcelain) {
>  		if (from)

Unrelated whitespace change in the method signature.

> @@ -687,7 +686,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
>  		break;
>  	case REF_STATUS_UPTODATE:
>  		print_ref_status('=', "[up to date]", ref,
> -						 ref->peer_ref, NULL, porcelain);
> +				 ref->peer_ref, NULL, porcelain);
>  		break;
>  	case REF_STATUS_REJECT_NONFASTFORWARD:
>  		print_ref_status('!', "[rejected]", ref, ref->peer_ref,

Unrelated whitespace change.

> @@ -711,8 +710,8 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
>  	return 1;
>  }
>  
> -static void print_push_status(const char *dest, struct ref *refs,
> -			      int verbose, int porcelain, int * nonfastforward)
> +void transport_print_push_status(const char *dest, struct ref *refs,
> +		  int verbose, int porcelain, int *nonfastforward)
>  {
>  	struct ref *ref;
>  	int n = 0;

Unrelated whitespace change for the second line of the signature.

-- 
Cheers,
Ray Chuan