Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1422714 (yank)
Summary: | Review Request: yank - tool for yanking (copying) stdin to clipboard | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Nemanja Milosevic <nmilosevnm> |
Component: | Package Review | Assignee: | Rex Dieter <rdieter> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | unspecified | ||
Version: | rawhide | CC: | besser82, germano.massullo, package-review, rc040203, rdieter, samuel-rhbugs |
Target Milestone: | --- | Keywords: | Reopened |
Target Release: | --- | Flags: | rdieter:
fedora-review+
|
Hardware: | All | ||
OS: | Linux | ||
URL: | https://github.com/mptre/yank | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | If docs needed, set a value | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2017-03-24 17:50:50 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: |
Description
Nemanja Milosevic
2017-02-16 00:28:50 UTC
Taken! *** You should use a link to the "raw" spec-file url… Fixing this quickly for you. Spec URL: https://pagure.io/yank-rpm/raw/master/f/yank.spec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/nmilosev/yank/fedora-25-x86_64/00512635-yank/yank-0.8.0-1.fc25.src.rpm Ah, sorry about the URL, didn't know it needs to be a direct link. Please let me know when you review the spec file, and what changes are needed. :) And thanks for your help. Cheers, Nemanja After reading a few spec files which are to be reviewed I saw some powerful stuff which I wasn't using. Like using 'install' instead of 'cp', and pulling sources from git automatically (spectool). So added all those to my .spec file: https://pagure.io/yank-rpm/raw/master/f/yank.spec Also pushed new build to COPR: https://copr.fedorainfracloud.org/coprs/nmilosev/yank/build/512881/ These were all small changes, but I want the spec to be as high quality as possible. Only thing I am unsure about in my spec file is about manual installation, please advise if I should use make_install or it isn't mandatory. Here is another version: https://pagure.io/yank-rpm/raw/makebuild/f/yank.spec It's a lot shorter, not sure which is more "clean". Let me know! :) * Why are you disabling debuginfo? .. %global debug_package %{nil} .. This in general is not helpful and therefore is not allowed. Please remove it. * Renaming binaries breaks debuginfo - Is this why you are disabling debuginfos? You probably need to rename the program's name to be compiled (ie. to change/patch the Makefile and change the program's name there) * The package does not use automake Please remove BuildRequires: automake * Building doesn't honor RPM_OPT_FLAGS. Please append CFLAGS=${RPM_OPT_FLAGS} to %make_build * The SOURCE0-URL is better written this way: Source0: %{url}/archive/v%{version}.tar.gz#/%{name}-%{version}.tar.gz Retrieving the package under this URL then will create a tarball named yank-0.8.0.tar.gz tarball and "v0.8.0.tar.gz" (In reply to Ralf Corsepius from comment #6) > * Why are you disabling debuginfo? > .. > %global debug_package %{nil} > .. > This in general is not helpful and therefore is not allowed. Please remove > it. > > * Renaming binaries breaks debuginfo - Is this why you are disabling > debuginfos? > Yes, that's why I did it. In the Makefile there was a install -s which stripped the information, so I wrote a patch to allow debuginfo to be generated. > You probably need to rename the program's name to be compiled (ie. to > change/patch the Makefile and change the program's name there) > Wrote a patch for this too, spec file is much cleaner now. :) > > * The package does not use automake > Please remove BuildRequires: automake > Oops, didn't notice that one, thank you! > * Building doesn't honor RPM_OPT_FLAGS. > Please append CFLAGS=${RPM_OPT_FLAGS} to %make_build > Sorry, didn't know about this. It's in a separate wiki page on the Packaging guidelines. Added now. > > * The SOURCE0-URL is better written this way: > Source0: %{url}/archive/v%{version}.tar.gz#/%{name}-%{version}.tar.gz > > Retrieving the package under this URL then will create a tarball named > yank-0.8.0.tar.gz tarball and "v0.8.0.tar.gz" Also added, thank you, didn't know you can do this! :) New spec file: https://pagure.io/yank-rpm/raw/makebuild/f/yank.spec New COPR build: https://copr.fedorainfracloud.org/coprs/nmilosev/yank/build/512925/ New SRPM: https://copr-be.cloud.fedoraproject.org/results/nmilosev/yank/fedora-25-x86_64/00512925-yank/yank-0.8.0-1.fc25.src.rpm Thank you so much for the review! Cheers, Nemanja Worked with the upstream to integrate both patches for the next release. Please see: https://github.com/mptre/yank/issues/35 https://github.com/mptre/yank/pull/36 Cheers, Nemanja Patches merged in the upstream! :) New version 0.8.1: SPEC file: https://pagure.io/yank-rpm/raw/0.8.1/f/yank.spec SRPM: https://copr-be.cloud.fedoraproject.org/results/nmilosev/yank/fedora-25-x86_64/00513904-yank/yank-0.8.1-1.fc25.src.rpm COPR build: https://copr.fedorainfracloud.org/coprs/nmilosev/yank/build/513904/ setting review flag per comment #1 Upstream update (small one) to 0.8.2 (Debian packager also made some changes): SPEC file: https://pagure.io/yank-rpm/raw/0.8.2/f/yank.spec SRPM: https://copr-be.cloud.fedoraproject.org/results/nmilosev/yank/fedora-25-x86_64/00514614-yank/yank-0.8.2-1.fc25.src.rpm COPR: https://copr.fedorainfracloud.org/coprs/nmilosev/yank/build/514614/ I'll continue the review, besser hasn't replied in awhile. naming: ok license: ok sources: ok 1af010fac3f5d594b9645d282e24d35b yank-0.8.2.tar.gz macros: ok scriplets: ok (n/a) rpmlint clean Fairly simple package, nicely done. APPROVED. I've sponsored you, welcome to fedora. Next steps: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers?rd=PackageMaintainers/Join#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/yank yank-0.8.2-2.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2017-9679fa4df4 yank-0.8.2-2.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-9679fa4df4 yank-0.8.2-2.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report. |