Bug 1067041
Summary: | Review Request: autodocksuite - AutoDock is a suite of docking tools to study protein-ligand interaction | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Mukundan Ragavan <nonamedotc> |
Component: | Package Review | Assignee: | Cole Robinson <crobinso> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | crobinso, package-review |
Target Milestone: | --- | Flags: | crobinso:
fedora-review+
gwync: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | autodocksuite-4.2.5.1-5.fc19 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2014-03-07 06:25:57 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
Mukundan Ragavan
2014-02-19 14:43:34 UTC
New SPEC URL: http://nonamedotc.fedorapeople.org/pkgreview/autodocksuite/autodocksuite.spec New SRPM URL: http://nonamedotc.fedorapeople.org/pkgreview/autodocksuite/autodocksuite-4.2.5.1-3.fc20.src.rpm koji scratch build with new srpm - http://koji.fedoraproject.org/koji/taskinfo?taskID=6547785 fedora-review - review.txt I ran to check - http://nonamedotc.fedorapeople.org/pkgreview/autodocksuite/review.txt I'll take this The version 4.2.5.1 is fine, it comes from upstream and RPM can handle it for comparisons. Besides the noted rpmlint spelling errors and fsf address, there's also this minor one: autodocksuite.src:24: W: mixed-use-of-spaces-and-tabs (spaces: line 24, tab: line 1) In my .vimrc, I have: set listchars=tab:>. which shows hard tabs as visible, but that's totally up to you. I'd recommend being consistent in the spec at least. Couple other points: - I don't think triggering ldconfig is required, the package isn't installing any shared libraries. - autodoc/COPYING is duplicated between the packages. Just stick it in the base package, since -doc requires the base package. I'd also stick the README in the -doc package but it's up to you. - In %build you have: export CFLAGS="%{optflags}" CXXFLAGS="%{optflags}" Does that make a difference? The %configure macro should do that for you. FYI I'm offline till Monday, sorry for bad timing. I'll follow up ASAP when I'm back Hi Cole, Thanks for your comments. (In reply to Cole Robinson from comment #3) > > Besides the noted rpmlint spelling errors and fsf address, there's also this > minor one: > > autodocksuite.src:24: W: mixed-use-of-spaces-and-tabs (spaces: line 24, tab: > line 1) > > In my .vimrc, I have: set listchars=tab:>. which shows hard tabs as > visible, but that's totally up to you. I'd recommend being consistent in the > spec at least. Fixed it. Actually, vimrc is a good suggestion. I will add it. > > Couple other points: > > - I don't think triggering ldconfig is required, the package isn't > installing any shared libraries. Done! > > - autodoc/COPYING is duplicated between the packages. Just stick it in the > base package, since -doc requires the base package. I'd also stick the > README in the -doc package but it's up to you. Done! README in -doc would be more appropriate. > > - In %build you have: > > export CFLAGS="%{optflags}" CXXFLAGS="%{optflags}" > > Does that make a difference? The %configure macro should do that for you. > Fixed. That was from a older spec file. Sorry. :) New SPEC URL: http://nonamedotc.fedorapeople.org/pkgreview/autodocksuite/autodocksuite.spec New SRPM URL: http://nonamedotc.fedorapeople.org/pkgreview/autodocksuite/autodocksuite-4.2.5.1-4.fc20.src.rpm My fedora-review on updated files (sorry, I did not do the [x] marks this time) - http://nonamedotc.fedorapeople.org/pkgreview/autodocksuite/review.txt The older review.txt is here - http://nonamedotc.fedorapeople.org/pkgreview/autodocksuite/ver2/review.txt 1. doc package doesn't have the strong reason to Requires: %{name}%{?_isa} = %{version}-%{release} So please remove this line. 2. cd autodock %configure make %{?_smp_mflags} cd %{_builddir}/src/autogrid %configure make %{?_smp_mflags} My optimization: pushd autodock %configure make %{?_smp_mflags} popd pushd autogrid %configure make %{?_smp_mflags} popd(this popd is optional) 3. %install cd autodock make install DESTDIR=%{buildroot} cd %{_builddir}/src/autogrid make install DESTDIR=%{buildroot} Mine: make -C autodock install DESTDIR=%{buildroot} make -C autogrid install DESTDIR=%{buildroot} Thank you Christopher! (In reply to Christopher Meng from comment #5) > 1. doc package doesn't have the strong reason to > > Requires: %{name}%{?_isa} = %{version}-%{release} > > So please remove this line. > Done! > 2. cd autodock > %configure > make %{?_smp_mflags} > > cd %{_builddir}/src/autogrid > %configure > make %{?_smp_mflags} > > My optimization: > > pushd autodock > %configure > make %{?_smp_mflags} > popd > > pushd autogrid > %configure > make %{?_smp_mflags} > popd(this popd is optional) Ok, I did not even know about pushd and popd. So, thanks a lot! Done. > > 3. %install > cd autodock > make install DESTDIR=%{buildroot} > > cd %{_builddir}/src/autogrid > make install DESTDIR=%{buildroot} > > Mine: > > make -C autodock install DESTDIR=%{buildroot} > make -C autogrid install DESTDIR=%{buildroot} Done. New SPEC URL: http://nonamedotc.fedorapeople.org/pkgreview/autodocksuite/autodocksuite.spec New SRPM URL: http://nonamedotc.fedorapeople.org/pkgreview/autodocksuite/autodocksuite-4.2.5.1-5.fc20.src.rpm My next question, do we really need a separate doc package? Reason? The doc file contains a user guide that is also available online (in the same form - pdf) and I just wanted to make it "optional". Also, rpmlint complains if I package it in the main package. Rpmlint sometimes can't reflect the needs from users. But your opinion is right. Fine. koji scratch build with the latest SRPM - http://koji.fedoraproject.org/koji/taskinfo?taskID=6566124 (i686 and arm done at the time of adding this comment) Files posted in Comment #6 look good to me Thanks Cole! New Package SCM Request ======================= Package Name: autodocksuite Short Description: AutoDock is a suite of docking tools to study protein-ligand interaction Owners: nonamedotc Branches: f19 f20 InitialCC: Git done (by process-git-requests). autodocksuite-4.2.5.1-5.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/autodocksuite-4.2.5.1-5.fc19 autodocksuite-4.2.5.1-5.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/autodocksuite-4.2.5.1-5.fc20 autodocksuite-4.2.5.1-5.fc19 has been pushed to the Fedora 19 testing repository. autodocksuite-4.2.5.1-5.fc20 has been pushed to the Fedora 20 stable repository. autodocksuite-4.2.5.1-5.fc19 has been pushed to the Fedora 19 stable repository. Package Change Request ====================== Package Name: autodocksuite New Branches: epel7 Owners: nonamedotc Git done (by process-git-requests). |