https://bugzilla.redhat.com/show_bug.cgi?id=1067041
Bug ID: 1067041 Summary: Review Request: autodocksuite - AutoDock is a suite of docking tools to study protein-ligand interaction Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: nonamedotc@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: http://nonamedotc.fedorapeople.org/pkgreview/autodocksuite/autodocksuite.spe... SRPM URL: http://nonamedotc.fedorapeople.org/pkgreview/autodocksuite/autodocksuite-4.2...
Description: AutoDock is a suite of automated docking tools. It is designed to predict \ how small molecules, such as substrates or drug candidates, bind to a \ receptor of known 3D structure. AutoDock 4 actually consists of two main \ programs: autodock performs the docking of the ligand to a set of grids \ describing the target protein; autogrid pre-calculates these grids. In \ addition to using them for docking, the atomic affinity grids can be \ visualized. This can help, for example, to guide organic synthetic chemists \ design better binders.
Fedora Account System Username: nonamedotc
* mock builds fine
koji scratch build here - http://koji.fedoraproject.org/koji/taskinfo?taskID=6547436
Question I have - Is the versioning fine for the package?
My own "review" - * There are a bunch of spelling error warnings in rpmlint (all of them are fine, as far as I can tell)
* incorrect-fsf-address errors: I will inform upstream
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
--- Comment #1 from Mukundan Ragavan nonamedotc@gmail.com --- New SPEC URL: http://nonamedotc.fedorapeople.org/pkgreview/autodocksuite/autodocksuite.spe...
New SRPM URL: http://nonamedotc.fedorapeople.org/pkgreview/autodocksuite/autodocksuite-4.2...
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
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
Cole Robinson crobinso@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |crobinso@redhat.com Assignee|nobody@fedoraproject.org |crobinso@redhat.com
--- Comment #2 from Cole Robinson crobinso@redhat.com --- I'll take this
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
--- Comment #3 from Cole Robinson crobinso@redhat.com --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
--- Comment #4 from Mukundan Ragavan nonamedotc@gmail.com --- 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.spe... New SRPM URL: http://nonamedotc.fedorapeople.org/pkgreview/autodocksuite/autodocksuite-4.2...
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
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
--- Comment #5 from Christopher Meng cickumqt@gmail.com --- 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}
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
--- Comment #6 from Mukundan Ragavan nonamedotc@gmail.com --- Thank you Christopher!
(In reply to Christopher Meng from comment #5)
- doc package doesn't have the strong reason to
Requires: %{name}%{?_isa} = %{version}-%{release}
So please remove this line.
Done!
- 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.
- %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.spe... New SRPM URL: http://nonamedotc.fedorapeople.org/pkgreview/autodocksuite/autodocksuite-4.2...
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
--- Comment #7 from Christopher Meng cickumqt@gmail.com --- My next question, do we really need a separate doc package? Reason?
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
--- Comment #8 from Mukundan Ragavan nonamedotc@gmail.com --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
--- Comment #9 from Christopher Meng cickumqt@gmail.com --- Rpmlint sometimes can't reflect the needs from users.
But your opinion is right.
Fine.
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
--- Comment #10 from Mukundan Ragavan nonamedotc@gmail.com --- 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)
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
Cole Robinson crobinso@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review+
--- Comment #11 from Cole Robinson crobinso@redhat.com --- Files posted in Comment #6 look good to me
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
--- Comment #12 from Mukundan Ragavan nonamedotc@gmail.com --- 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:
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
Mukundan Ragavan nonamedotc@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
Jon Ciesla limburgher@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-cvs? |fedora-cvs+
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
--- Comment #13 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
--- Comment #14 from Fedora Update System updates@fedoraproject.org --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
--- Comment #15 from Fedora Update System updates@fedoraproject.org --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #16 from Fedora Update System updates@fedoraproject.org --- autodocksuite-4.2.5.1-5.fc19 has been pushed to the Fedora 19 testing repository.
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Fixed In Version| |autodocksuite-4.2.5.1-5.fc2 | |0 Resolution|--- |ERRATA Last Closed| |2014-03-07 01:25:57
--- Comment #17 from Fedora Update System updates@fedoraproject.org --- autodocksuite-4.2.5.1-5.fc20 has been pushed to the Fedora 20 stable repository.
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|autodocksuite-4.2.5.1-5.fc2 |autodocksuite-4.2.5.1-5.fc1 |0 |9
--- Comment #18 from Fedora Update System updates@fedoraproject.org --- autodocksuite-4.2.5.1-5.fc19 has been pushed to the Fedora 19 stable repository.
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
--- Comment #19 from Mukundan Ragavan nonamedotc@gmail.com --- Package Change Request ====================== Package Name: autodocksuite New Branches: epel7 Owners: nonamedotc
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
Mukundan Ragavan nonamedotc@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-cvs+ |fedora-cvs?
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
Jon Ciesla limburgher@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-cvs? |fedora-cvs+
https://bugzilla.redhat.com/show_bug.cgi?id=1067041
--- Comment #20 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
package-review@lists.fedoraproject.org