From: Prarit Bhargava on gitlab.com https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1757#note_944835271 >- [3/17] `030f50682c27` ("redhat/Makefile: Add kernel-NVR comment") > > Is there any reason to make RELEASETAG variable a recursively expanded variable instead of simply expanded variables, that is, using ```=``` assignment instead of ```:=```? (See eg. https://ftp.gnu.org/old- gnu/Manuals/make-3.79.1/html_chapter/make_toc.html#TOC59). Mostly curious because it should work either way. > No reason. For consistency I've changed this to ":=". >- [6/17] `df36fcdd089d` ("redhat/Makefile: Rename RPMVERSION to BASEVERSION") > > You also need to change rh-dist-git.sh script to use BASEVERSION: > > ``` > redhat/scripts/rh-dist-git.sh:echo -e "${PACKAGE_NAME}-${RPMVERSION}\n" > "$tmpdir"/changelog > ``` Fixed. > >- [7/17] `34fe4a6a439c` ("redhat/Makefile: General cleanup") > > Probably you want the changelog here to be something like "Add comment about the SPECTARFILE_RELEASE variable", instead of "General cleanup", because it doesn't look a cleanup. Fixed. > >- [9/17] `54a3fefd5b39` ("redhat/kernel.spec.template: Move genspec variables into one section") > > May add to the changelog here that you're also adding back the kversion variable that maps to SPECKVERSION Fixed. > >- [10/17] `357ddd348f6c` ("redhat/genspec.sh: Remove SPECBUILDID") > > I think you need to drop this change, it is not true that SPECBUILD is not used. It is used in the SPECRELEASE variable: > > ``` > redhat/genspec.sh:SPECRELEASE="${UPSTREAMBUILD}""${BUILD}""%{?buildid}%{?di st}" > ``` > > This is replaced/used in the spec file. I've changed this commit to add a comment about SPECBUILDID usage. > > I remember buildid could be edited when building/patching directly in dist- git, > support folks from CEE used to do test kernel builds for customers using the > buildid variable in the spec. > >- [13/17] `274beb50dfe2` ("redhat/Makefile: Reorganize MARKER code") > > I think you could go further here and do the assignment of MARKER once at the end of if/endif condition and do ```MARKER:=v$(UPSTREAM_TARBALL_NAME)``` once. Also any reason of using ```=``` instead of ```:=``` as assignment operator? (see my other comment about the difference of it in variable expansion in make) > Y'know, I thought about doing this but wasn't sure if the end result was too messy. I've changed it to do what you are suggesting. Hopefully it makes sense. >- [14/17] `5fcbbc8fe262` ("redhat/Makefile: Fix dist-brew & distg-brew targets") > > I think you missed a ```-``` here, ```--scratch``` instead of ```-scratch```, in both targets. Fixed. > >- [15/17] `7b1de039af6d` ("redhat/Makefile: Rename BUILDID to LOCALVERSION") > > Should we also change redhat/koji/Makefile and rename BUILDID there too? I thought about this and decided not to make this change. The reason is that the BUILDID in that Makefile really references the 'buildid' variable set with 'git notes --ref buildid -m ".buildid" HEAD'. IOW, this isn't really a LOCALVERSION. _______________________________________________ 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 Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
