#26 Add --name flag to set goname and spec name
Merged by gotmax23. Opened by mikelo2.
GoSIG/ mikelo2/go2rpm goname  into  master

Download 26.patch
no initial comment

This PR would need to be rebased once https://pagure.io/GoSIG/go2rpm/pull-request/25 is merged. I'm creating this PR before to get some comments, if any.

rebased onto f70d700509146dd7beb7ee54f96079c0533634bd

Rebased.

{% if not 'golang' in goname %}

I'm not a fan of this conditional. It relies on heuristics that might change. Also, what happens if a user sets --name to a value that includes golang? Can you add a separate goname variable that is None by default and change this to something like:

+{% if goname %}
+%global goname {{ goname }}
+{% endif %}

In the long term, I'd like to get rid of rpmname() and instead use rpmspec -q --qf '%{name}\n' --srpm SPECFILE to figure out the name. This way, we won't have to duplicate code from go-rpm-macros.

I think action="store" is redundant.

I'm not a fan of this conditional. It relies on heuristics that might change. Also, what happens if a user sets --name to a value that includes golang? Can you add a separate goname variable that is None by default and change this to something like:

IIRC goname is always set in https://pagure.io/GoSIG/go2rpm/blob/master/f/go2rpm/main.py#_681 but I could declare a new variable like "custoname" and check that.

    kwargs["custonname"] = args.name

And then in the template:

{% if  custonname is not none %}
%global goname {{ goname }}
{% endif %}

rebased onto b61bdc79b8d19f86b2d7ab7eca831ac733d98692

rebased onto c2e86b9e2a04e9f4a0152b041c602da6d4939456

Hmm, it doesn't seem like {{ goname }} was used at all in the template before, so it should be okay to change kwargs["goname"] to args.name and have it be None if --name wasn't passed.


Try this:

diff --git a/go2rpm/__main__.py b/go2rpm/__main__.py
index ceb6501..d4cd835 100644
--- a/go2rpm/__main__.py
+++ b/go2rpm/__main__.py
@@ -664,7 +664,7 @@ def main():
     license_files = get_license_files(git_local_path)
     doc_files = get_doc_files(git_local_path)
-    if args.name is not None:
+    if args.name:
         name = args.name
     else:
         name = rpmname(goipath + subdir)
@@ -685,8 +685,7 @@ def main():
     kwargs = {}
     kwargs["generator_version"] = __version__
     kwargs["goipath"] = goipath
-    kwargs["goname"] = name
-    kwargs["custonname"] = args.name
+    kwargs["goname"] = args.name
     kwargs["forge"] = forge
     kwargs["subdir"] = subdir
     kwargs["altipaths"] = args.altipaths
diff --git a/go2rpm/templates/profile2.spec b/go2rpm/templates/profile2.spec
index c9e03be..dd78788 100644
--- a/go2rpm/templates/profile2.spec
+++ b/go2rpm/templates/profile2.spec
@@ -34,8 +34,8 @@ Version:                {{ tag }}
 {% endif %}
 %gometa -f
-{% if custonname is not none %}
-%global goname {{ goname }}
+{% if goname %}
+%global goname          {{ goname }}
 {% endif %}
 {% if altipaths is not none %}

rebased onto e6dc566387dde820946810b1b52694dfa186c45a

You're right, as goname is not used in the template it can be that simple.

Missing trailing comma

if args.name:

rebased onto 71a215f2347d39813bc1b281fcc8b7145a51bbf5

I guess Pagure doesn't send notifications when you push to a PR, so I didn't see your changes earlier.

Anyways, this looks good to me now. Thanks!

Pull-Request has been merged by gotmax23

Thanks for your time reviewing this PR @gotmax23

Metadata