diff mbox series

fixup! upload-pack: change multi_ack to an enum

Message ID 20200602190521.32877-1-jonathantanmy@google.com (mailing list archive)
State New, archived
Headers show
Series fixup! upload-pack: change multi_ack to an enum | expand

Commit Message

Jonathan Tan June 2, 2020, 7:05 p.m. UTC 
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
I think enum values should be all-caps, so here is a fixup for that. I
also fixed a spacing issue (2 spaces between "enum" and "{").

Also, maybe replace the first paragraph of the 1st patch:

  As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
  more thoroughly, let's actually start using some bitfields of
  that struct, which were previously unused.

with:

  As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
  more thoroughly, let's actually start using some bitfields of
  that struct. These bitfields were introduced in 3145ea957d
  ("upload-pack: introduce fetch server command", 2018-03-15) but were
  never used.

Other than that, this patch set looks good to me.

 upload-pack.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Christian Couder June 2, 2020, 7:28 p.m. UTC | #1 
On Tue, Jun 2, 2020 at 9:05 PM Jonathan Tan <jonathantanmy@google.com> wrote:
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---

Thanks for the patch!

> I think enum values should be all-caps, so here is a fixup for that.

It's true that they are often all-caps, but there are a number of
places where some enum values are lower case like:

- apply.h
- dir.c
- pretty.c
...

> I also fixed a spacing issue (2 spaces between "enum" and "{").

Thanks for that.

> Also, maybe replace the first paragraph of the 1st patch:
>
>   As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
>   more thoroughly, let's actually start using some bitfields of
>   that struct, which were previously unused.
>
> with:
>
>   As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
>   more thoroughly, let's actually start using some bitfields of
>   that struct. These bitfields were introduced in 3145ea957d
>   ("upload-pack: introduce fetch server command", 2018-03-15) but were
>   never used.

Yeah, it seems better.

> Other than that, this patch set looks good to me.

Thank you for your review. I am ok to resend a V3 of the part 2 patch
series with all these changes. If having all-caps enum values is not
required though, I wonder if it's worth resending everything.
diff mbox series

Patch

diff --git a/upload-pack.c b/upload-pack.c
index 30e8c54060..bc7e3ca19d 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -74,10 +74,10 @@  struct upload_pack_data {
 	int keepalive;
 
 	unsigned int timeout;					/* v0 only */
-	enum  {
-		no_multi_ack = 0,
-		multi_ack = 1,
-		multi_ack_detailed = 2
+	enum {
+		NO_MULTI_ACK = 0,
+		MULTI_ACK = 1,
+		MULTI_ACK_DETAILED = 2
 	} multi_ack;						/* v0 only */
 
 	/* 0 for no sideband, otherwise DEFAULT_PACKET_MAX or LARGE_PACKET_MAX */
@@ -451,7 +451,7 @@  static int get_common_commits(struct upload_pack_data *data,
 		reset_timeout(data->timeout);
 
 		if (packet_reader_read(reader) != PACKET_READ_NORMAL) {
-			if (data->multi_ack == multi_ack_detailed
+			if (data->multi_ack == MULTI_ACK_DETAILED
 			    && got_common
 			    && !got_other
 			    && ok_to_give_up(&data->have_obj, &data->want_obj)) {
@@ -478,7 +478,7 @@  static int get_common_commits(struct upload_pack_data *data,
 				if (data->multi_ack
 				    && ok_to_give_up(&data->have_obj, &data->want_obj)) {
 					const char *hex = oid_to_hex(&oid);
-					if (data->multi_ack == multi_ack_detailed) {
+					if (data->multi_ack == MULTI_ACK_DETAILED) {
 						sent_ready = 1;
 						packet_write_fmt(1, "ACK %s ready\n", hex);
 					} else
@@ -488,7 +488,7 @@  static int get_common_commits(struct upload_pack_data *data,
 			default:
 				got_common = 1;
 				oid_to_hex_r(last_hex, &oid);
-				if (data->multi_ack == multi_ack_detailed)
+				if (data->multi_ack == MULTI_ACK_DETAILED)
 					packet_write_fmt(1, "ACK %s common\n", last_hex);
 				else if (data->multi_ack)
 					packet_write_fmt(1, "ACK %s continue\n", last_hex);
@@ -968,9 +968,9 @@  static void receive_needs(struct upload_pack_data *data,
 		if (parse_feature_request(features, "deepen-relative"))
 			data->deepen_relative = 1;
 		if (parse_feature_request(features, "multi_ack_detailed"))
-			data->multi_ack = multi_ack_detailed;
+			data->multi_ack = MULTI_ACK_DETAILED;
 		else if (parse_feature_request(features, "multi_ack"))
-			data->multi_ack = multi_ack;
+			data->multi_ack = MULTI_ACK;
 		if (parse_feature_request(features, "no-done"))
 			data->no_done = 1;
 		if (parse_feature_request(features, "thin-pack"))