| Message ID | cover.1742250259.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | refspec: treat 'fetch' as a Boolean value | expand |
Taylor Blau <me@ttaylorr.com> writes: > This series explores treating the 'fetch' field of 'struct refspec' as a > Boolean, instead of taking on values called "REFSPEC_FETCH" and > "REFSPEC_PUSH". > > The refactoring takes place in a couple of steps, but IMHO the end > result is pleasing and an improvement in clarity over the existing API. > > For reference, in one of the patches above, I had suggested replacing > something like: > > if (refspec->fetch == REFSPEC_FETCH) { > /* something */ > } else { > /* something else */ > } > > with a switch/case-statement, only to realize that 'refspec->fetch' is > really a Boolean in disguise. > > I am not overly attached to these patches one way or the other. I think > they do improve the refspec.h API, but they're also ripe for > bike-shedding. So if we agree that they are good, I'm happy to see them > move forward. But if not, I'd rather not spend the list's time > bike-shedding here, so I'd just as soon drop them. > > Thanks in advance for your review! > > Taylor Blau (4): > refspec: treat 'fetch' as a Boolean value > refspec: replace `refspec_init()` with fetch/push variants > refspec: remove refspec_item_init_or_die() > refspec: replace `refspec_item_init()` with fetch/push variants > > builtin/fetch.c | 2 +- > builtin/pull.c | 3 ++- > git-diff-pairs | Bin 0 -> 12166200 bytes > refspec.c | 38 +++++++++++++++++++++++++++----------- > refspec.h | 18 +++++++----------- > remote.c | 4 ++-- > transport-helper.c | 2 +- > 7 files changed, 40 insertions(+), 27 deletions(-) > create mode 100755 git-diff-pairs Somebody does not proofread their cover letter. Sheesh ;-)
On Mon, Mar 17, 2025 at 04:26:28PM -0700, Junio C Hamano wrote: > > builtin/fetch.c | 2 +- > > builtin/pull.c | 3 ++- > > git-diff-pairs | Bin 0 -> 12166200 bytes > > refspec.c | 38 +++++++++++++++++++++++++++----------- > > refspec.h | 18 +++++++----------- > > remote.c | 4 ++-- > > transport-helper.c | 2 +- > > 7 files changed, 40 insertions(+), 27 deletions(-) > > create mode 100755 git-diff-pairs > > Somebody does not proofread their cover letter. Sheesh ;-) Ugh, for goodness sake. Sorry about that, I'll send another round. Thanks, Taylor
This series explores treating the 'fetch' field of 'struct refspec' as a Boolean, instead of taking on values called "REFSPEC_FETCH" and "REFSPEC_PUSH". The refactoring takes place in a couple of steps, but IMHO the end result is pleasing and an improvement in clarity over the existing API. For reference, in one of the patches above, I had suggested replacing something like: if (refspec->fetch == REFSPEC_FETCH) { /* something */ } else { /* something else */ } with a switch/case-statement, only to realize that 'refspec->fetch' is really a Boolean in disguise. I am not overly attached to these patches one way or the other. I think they do improve the refspec.h API, but they're also ripe for bike-shedding. So if we agree that they are good, I'm happy to see them move forward. But if not, I'd rather not spend the list's time bike-shedding here, so I'd just as soon drop them. Thanks in advance for your review! Taylor Blau (4): refspec: treat 'fetch' as a Boolean value refspec: replace `refspec_init()` with fetch/push variants refspec: remove refspec_item_init_or_die() refspec: replace `refspec_item_init()` with fetch/push variants builtin/fetch.c | 2 +- builtin/pull.c | 3 ++- git-diff-pairs | Bin 0 -> 12166200 bytes refspec.c | 38 +++++++++++++++++++++++++++----------- refspec.h | 18 +++++++----------- remote.c | 4 ++-- transport-helper.c | 2 +- 7 files changed, 40 insertions(+), 27 deletions(-) create mode 100755 git-diff-pairs base-commit: c702dd48567cfebca3d4a06b691de97da3f8dc4a