Re: [OS-BUILD PATCHv12 1/5] [redhat] Add GIT macro to Makefile and Makefile.common:
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(a)redhat.com>
>
> 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