Bug 1795469 - Review Request: string-template-maven-plugin - Execute StringTemplate files during a maven build
Summary: Review Request: string-template-maven-plugin - Execute StringTemplate files d...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 1795468
Blocks: 1795470
TreeView+ depends on / blocked
 
Reported: 2020-01-28 04:09 UTC by Jerry James
Modified: 2020-02-03 20:38 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-02-03 20:38:25 UTC
Type: ---
Embargoed:
decathorpe: fedora-review+


Attachments (Terms of Use)

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.


Note You need to log in before you can comment on or make changes to this bug.