git.vger.kernel.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).