Re: [OS-BUILD PATCHv12 1/5] [redhat] Add GIT macro to Makefile and Makefile.common:

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 23, 2020 at 12:02:44PM -0300, Herton R. Krzesinski wrote:
> On Fri, Nov 20, 2020 at 12:17:02AM -0000, GitLab Bridge on behalf of bcrocker wrote:
> > From: Ben Crocker <bcrocker@xxxxxxxxxx>
> > 
> > GIT ?= git
> 
> Hi Ben I have some comments, please see them below and inline in the code.
> 
> > 
> > and replace literal occurrences of 'git' with $(GIT).
> > This change enables us to override 'git' with, e.g., some
> > arbitrary shell script that prints additional information
> > and/or does additional processing before and/or after (or
> > even instead of) invoking /usr/bin/git.
> > 
> > Add dist-dump-variables for dynamically deriving variables
> > from Makefile.common and dumping them.
> > 
> > Add a dist-clean-scripts target to clean up generated scripts.
> 
> I would split the addition of the dist-dump-variables in a new patch, because
> it's not related to the git replacement, it's a different topic. The
> dist-clean-scripts change is related to the dist-dump-variables, so it can be in
> the same dist-dump-variables new patch.

Actually you can move dist-dump-variables stuff to patch 2 as well, since it's
where it's use is introduced, no need for a new patch.

I just had these comments, other than that the remaining patches/changes looks
good to me.

> 
> > 
> > Add a description of the new dist-self-test target to dist-full-help.
> 
> You are adding the dist-self-test help in this patch, instead of patch 2,
> which really adds the dist-self-test target. I think you should move this to
> patch 2.
> 
(...)

- 
[]'s
Herton
_______________________________________________
kernel mailing list -- kernel@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to kernel-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/kernel@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora General Discussion]     [Older Fedora Users Archive]     [Fedora Advisory Board]     [Fedora Security]     [Fedora Devel Java]     [Fedora Legacy]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Mentors]     [Fedora Package Announce]     [Fedora Package Review]     [Fedora Music]     [Fedora Packaging]     [Centos]     [Fedora SELinux]     [Coolkey]     [Yum Users]     [Tux]     [Yosemite News]     [KDE Users]     [Fedora Art]     [Fedora Docs]     [USB]     [Asterisk PBX]

  Powered by Linux