On Fri, Jun 14, 2024 at 3:32 AM Jeff King <peff@xxxxxxxx> wrote: > [...] > There are two spots to pay special attention to here: > > 1. in builtin/remote.c's get_url(), we are selecting first based on > push_mode and then falling back to "url" when we're in push_mode > but no pushurl is defined. The updated code makes that much more > clear, compared to the original which had an "else" fall-through. > > 2. likewise in that file's set_url(), we _only_ respect push_mode, > sine the point is that we are adding to pushurl in that case s/sine/since/ > (whether it is empty or not). And thus it does not use our helper > function. > > Signed-off-by: Jeff King <peff@xxxxxxxx> > --- > builtin/push.c | 21 ++++----------- > builtin/remote.c | 69 ++++++++++++++---------------------------------- > remote.c | 5 ++++ > remote.h | 1 + > 4 files changed, 31 insertions(+), 65 deletions(-) > [...] I like the simplifications in this patch; it all looks good to me.