Subject: Re: [PATCH v3 05/10] bulk-checkin: extract abstract `bulk_checkin_source`

From: Taylor Blau <[email-masked]>
Date: Thu, 19 Oct 2023 11:19:23 -0400

On Wed, Oct 18, 2023 at 04:10:43PM -0700, Junio C Hamano wrote:
> Looks OK, even though I expected to see a bit more involved object
> orientation with something like
>
> 	struct bulk_checkin_source {
> 		off_t (*read)(struct bulk_checkin_source *, void *, size_t);
> 		off_t (*seek)(struct bulk_checkin_source *, off_t);
> 		union {
> 			struct {
> 				int fd;
> 				size_t size;
> 				const char *path;
> 			} from_fd;
> 			struct {
> 				...
> 			} incore;
> 		} data;
> 	};
>
> As there will only be two subclasses of this thing, it may not
> matter all that much right now, but it would be much nicer as your
> methods do not have to care about "switch (enum) { case FILE: ... }".

I want to be cautious of going too far in this direction. I anticipate
that "two" is probably the maximum number of kinds of sources we can
reasonably expect for the foreseeable future. If that changes, it's easy
enough to convert from the existing implementation to something closer
to the above.

Thanks,
Taylor