All of lore.kernel.org
 help / color / mirror / Atom feed
From: Felipe Contreras <felipe.contreras@gmail.com>
To: Junio C Hamano <gitster@pobox.com>,
	Felipe Contreras <felipe.contreras@gmail.com>
Cc: Jeff King <peff@peff.net>, git@vger.kernel.org
Subject: Re: [PATCH] doc: set actual revdate for manpages
Date: Fri, 14 Apr 2023 12:26:13 -0600	[thread overview]
Message-ID: <64399ac5685a4_5f77329495@chronos.notmuch> (raw)
In-Reply-To: <xmqqildys97o.fsf@gitster.g>

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> >> > diff --git a/Documentation/Makefile b/Documentation/Makefile
> >> > index 3133ea3182..b629176d7d 100644
> >> > --- a/Documentation/Makefile
> >> > +++ b/Documentation/Makefile
> >> > @@ -144,13 +144,16 @@ man5dir = $(mandir)/man5
> >> >  man7dir = $(mandir)/man7
> >> >  # DESTDIR =
> >> >  
> >> > +GIT_DATE := $(shell git show --quiet --pretty='%as')
> >> 
> >> What will/should this do in a distribution tarball, where we won't have
> >> a Git repository at all? I think we'll just end up with a blank date in
> >> the xml file, though it looks like docbook turns that into today's date
> >> in the result.
> >> 
> >> That's not _too_ bad, but feels a bit inconsistent (and it uses the
> >> format you're trying to get rid of!).
> >> 
> >> It would be nicer to populate the date variable in that case, like we do
> >> for GIT_VERSION. I think that could look something like this:
> >
> > Indeed, that would be better, but that totally can be done in a separate patch,
> > or a separate series even.
> 
> Seeing Peff's change, it sounds so small a thing that it feels a bit
> unnatural not to do that in the same series, at least to me.

It should be a small thing, but the GIT-VERSION-GEN script has not been
updated since 2008 and has a lot of issues (from my point of view).

I think it should be cleaned up first (I already sent a patch series for
that), *then* it would be trivial to add another field.

But that would add another (unncessary) dependency to this series.

> Having said that, I think that "we make progress one step at a time"
> is perfectly acceptable and may even be preferred, as long as the
> formatted manpages from the tarball would not change between with
> and without this patch.  That way, we make the output from a
> repository better while keeping the output from a tarball extract
> intact, and make the latter match the former in a later effort.
> 
> So, I "wasted" (not really---this was a fruitful validation that is
> a part of a review process) some time to play with this on top of
> 'seen' to see how well the tarball extract fares.  We get an error
> message from "git show" complaining about "not a git repository" but
> that is to be expected ("sh GIT-VERSION-GEN" does not share the
> problem, though).
> 
> At least with the versions of toolchain picked by default in my
> environment, I seem to be getting identical "04/14/2023" in a
> directory extracted out of a tarball taken from 'seen' (with and
> without this patch) in the formatted manpages, because we do not
> have any record in the input either way.
> 
> Formatted output from a repository working tree changes from
> "04/14/2023" to "2023-04-13".  The value change may be intended, but
> I am not sure if the format change was intended or even welcome.

I live in the vast majority of countries where MM/DD/YYYY makes
absolutely no sense, so I think it's a plus if any man pages have a sane
date.

I'm pretty sure I'm not alone on this, but some might disagree.

> If we want to correct the date format, it can totally be done in a
> separate patch, or a separate series even, with some justification in
> the proposed log message, I think.

Sure, but in order to do that we would need a way to convert the sane
standard git dates to USA-centric dates, and I'm not so sure the
tidyness of having both dates wrong is worth it.

Would you consider something like this more welcome?

  GIT_DATE := $(shell git show --quiet --pretty='%as' | sed -e 's|^\(.\+\)-\(.\+\)-\(.\+\)$|\2/\3/\1|'

-- 
Felipe Contreras

  parent reply	other threads:[~2023-04-14 18:26 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-13  7:47 [PATCH] doc: set actual revdate for manpages Felipe Contreras
2023-04-14  7:04 ` Jeff King
2023-04-14 12:40   ` Felipe Contreras
2023-04-14 16:46     ` Junio C Hamano
2023-04-14 17:14       ` Junio C Hamano
2023-04-14 18:40         ` Felipe Contreras
2023-04-14 18:26       ` Felipe Contreras [this message]
2023-04-14 21:45       ` Jeff King
2023-04-14 22:06         ` Junio C Hamano
2023-04-14 21:35     ` Jeff King
2023-04-14 15:27   ` Junio C Hamano
2023-04-14 16:20     ` Felipe Contreras
2023-04-14 17:36       ` Junio C Hamano
2023-04-14 18:59         ` Felipe Contreras

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=64399ac5685a4_5f77329495@chronos.notmuch \
    --to=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.