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:
--name
golang
goname
+{% 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.
rpmname()
rpmspec -q --qf '%{name}\n' --srpm SPECFILE
go-rpm-macros
I think action="store" is redundant.
action="store"
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.
{{ goname }}
kwargs["goname"]
args.name
None
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