Bug 1795469

Summary:	Review Request: string-template-maven-plugin - Execute StringTemplate files during a maven build
Product:	[Fedora] Fedora	Reporter:	Jerry James <loganjerry>
Component:	Package Review	Assignee:	Fabio Valentini <decathorpe>
Status:	CLOSED NEXTRELEASE	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	decathorpe, package-review, zbyszek
Target Milestone:	---	Flags:	decathorpe: fedora-review+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2020-02-03 20:38:25 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:	1795468
Bug Blocks:	1795470

Description Jerry James 2020-01-28 04:09:36 UTC

Spec URL: https://jjames.fedorapeople.org/string-template-maven-plugin/string-template-maven-plugin.spec
SRPM URL: https://jjames.fedorapeople.org/string-template-maven-plugin/string-template-maven-plugin-1.1-1.fc32.src.rpm
Fedora Account System Username: jjames
Description: This plugin allows you to execute StringTemplate template files during your build.  The values for templates can come from static declarations or from a Java class specified to be executed.

Comment 1 Fabio Valentini 2020-01-28 12:11:54 UTC

Looks like I was too late for the other two :) But once they are built, I'll review this package and antlr4.

Comment 2 Zbigniew Jędrzejewski-Szmek 2020-01-30 08:47:33 UTC

My suggestion would be to simply build and install the dependency in mock, and
then review this one using "mock -o ' -n' -b 1795469" or similar.

Apart from saving time, this has the advantage that it provides an additional test
for those dependencies. It is possible that something was missed in review (e.g. some
forgotten provides or a runtime issue), and building this package on top might allow
us to catch those bugs earlier than we would otherwise.

Comment 3 Fabio Valentini 2020-01-30 12:03:49 UTC

Manual review below (with dependent packages built and installed in mock manually):

- License is correct and permissible
- Package builds and installs correctly
- Package conforms to (Java) Packaging Guidelines
- Patches are documented and justified
- rpmlint shows no issues

================
Package APPROVED
================

Non-blocking suggestion:
I'm pretty sure BR: maven-source-plugin is not actually necessary since the build doesn't produce source JARs.
You should be able to remove the BR from the build, as well (with %pom_remove_plugin :maven-source-plugin").
It's not strictly necessary to do that, but it makes your package's dependency tree smaller and probably makes mock/koji builds marginally faster.

Comment 4 Fabio Valentini 2020-01-30 12:42:14 UTC

Ah, one thing I forgot:

It looks like the last upstream release doesn't contain a COPYING / LICENSE file (it's there in git master, but git hasn't been touched since 2013).
However, it looks like the README.md file contains the text of the actual license. You should probably add "%license README.md" in this case.

Comment 5 Jerry James 2020-02-03 18:41:09 UTC

Thanks for the suggestions.  I will apply both of those when I import the package.  Thank you for the review!

Comment 6 Gwyn Ciesla 2020-02-03 18:45:44 UTC

(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/string-template-maven-plugin

Comment 7 Jerry James 2020-02-03 20:38:25 UTC

Built in Rawhide.