Hi Junio,
On Tue, 24 Mar 2020, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
> writes:
>
> > + Note: this global header might contain useful information. For example,
> > + in the output of `git archive`, it lists the original commit, which _is_
> > + useful information. A future improvement to the `import-tars.perl`
> > + script might be to include that information in the commit message, or do
> > + other things with the information (e.g. use `mtime` information
> > + contained in the global header as date of the commit). This patch does
> > + not prevent any future patch from making that happen, it only prevents
> > + the header from being treated as if it was a regular file.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> >
> >
> >
> > contrib/fast-import/import-tars.perl | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/contrib/fast-import/import-tars.perl b/contrib/fast-import/import-tars.perl
> > index e800d9f5c9c..d50ce26d5d9 100755
> > --- a/contrib/fast-import/import-tars.perl
> > +++ b/contrib/fast-import/import-tars.perl
> > @@ -139,6 +139,8 @@
> > print FI "\n";
> > }
> >
> > + next if ($typeflag eq 'g'); # ignore global header
> > +
>
> Yeah, it is more like "don't create a phony file out of global
> header" which is exactly the point of this fix, rather than "ignore
> global header", and if the contents of the header is used for any
> other purpose (e.g. metadata that will be added to the log message
> or mode bits that is forced on all files), that can be handled
> before this line. And the current code structure is already
> prepared for it: all that remains in the block after this point is
> to create a file at $path and store its contents.
>
> So, this makes sense. I suspect that with an update to the comment
> in the direction, there probably is no need for the huge "Note" in
> the log message.
Too many of such in-code comments added by me became stale. I'd rather
keep this in the commit message where it explains the reasoning for the
current change.
Thanks,
Dscho
>
> Thanks.
>
> > my $path;
> > if ($prefix) {
> > $path = "$prefix/$name";
> >
> > base-commit: b4374e96c84ed9394fed363973eb540da308ed4f
>
