Bug 683790 - Review Request: rubygem-hoe-yard - A Hoe plug-in for generating YARD documentation
Summary: Review Request: rubygem-hoe-yard - A Hoe plug-in for generating YARD document...
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Bohuslav "Slavek" Kabrda
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 683789
TreeView+ depends on / blocked
 
Reported: 2011-03-10 11:37 UTC by Jan Klepek
Modified: 2015-07-02 09:46 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-07-02 09:46:43 UTC
Type: ---
bkabrda: fedora-review?


Attachments (Terms of Use)

Description Jan Klepek 2011-03-10 11:37:44 UTC
Spec URL: http://hpejakle.fedorapeople.org/packages/rubygem-hoe-yard.spec
SRPM URL: http://hpejakle.fedorapeople.org/packages/rubygem-hoe-yard-0.1.2-1.fc14.src.rpm
Description: A Hoe plug-in for generating YARD documentation.
Using the Hoe YARD plug-in, projects can begin generating YARD documentation
instantly. Additionally, any resulting RubyGems will be properly configured
to automatically generate YARD documentation upon installation.


rpmlint: 
rubygem-rubytree.noarch: W: hidden-file-or-dir /usr/lib/ruby/gems/1.8/gems/rubytree-0.8.1/.yardoc
The file or directory is hidden. You should see if this is normal, and delete
it from the package if not.

rubygem-rubytree.noarch: W: hidden-file-or-dir /usr/lib/ruby/gems/1.8/gems/rubytree-0.8.1/.yardoc
The file or directory is hidden. You should see if this is normal, and delete
it from the package if not.

2 packages and 0 specfiles checked; 0 errors, 2 warnings.

Comment 1 Jan Klepek 2011-03-10 11:43:38 UTC
koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2900098

Comment 2 Bohuslav "Slavek" Kabrda 2011-10-14 11:37:13 UTC
I'm taking this one.

Comment 3 Bohuslav "Slavek" Kabrda 2011-10-14 11:51:23 UTC
- License is MIT.
- The defined %ruby_sitelib macro is never used, so please remove it.
- You should use %global instead of %define, according to [1].
- You don't need to specify BuildRoot tag, see [2]
- Could you explain the Requires: rubygem(rubyforge) and Requires: rubygem(gemcutter)? I don't see why the library wouldn't work without them.
- Simlarly, can you explain the BuildRequires: rubygem(yard)? I don't see what you need yard for during the build.
- You don't need to use the "%defattr(-, root, root, -)" line, see [3]
- Consider moving documentation into a subpackage (except of README.rdoc, which contains licensing info and therefore should be left in the main package).
- rpmbuild complains about History.rdoc and Manifest.txt listed twice. To solve it:
** the package shouldn't own the whole %{gemdir}/gems/%{gemname}-%{version}/ directory (BTW you can use %{geminstdir} instead of it), but should rather own %dir %{geminstdir}
** when you do that, you will need to add every subdirectory and file in %{geminstdir} to %files, but you will be able to avoid the complaints about files listed twice

[1] https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define
[2] https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag
[3] https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions

Comment 4 Bohuslav "Slavek" Kabrda 2011-10-24 05:55:04 UTC
Ping, any progress on this one?

Comment 5 Jan Klepek 2011-10-26 12:22:42 UTC
Hi Bohuslav,

Yes, it is on my radar, I expect that I could move with this during weekend/next week.

Comment 6 Jan Klepek 2011-11-17 16:40:32 UTC
(In reply to comment #3)
> - License is MIT.
- Fixed

> - The defined %ruby_sitelib macro is never used, so please remove it.
- Fixed

> - You should use %global instead of %define, according to [1].
- Fixed

> - You don't need to specify BuildRoot tag, see [2]
- I know, as there is still open possibility to have this for EPEL in future, I keep it there.

> - Could you explain the Requires: rubygem(rubyforge) and Requires:
> rubygem(gemcutter)? I don't see why the library wouldn't work without them.
> - Simlarly, can you explain the BuildRequires: rubygem(yard)? I don't see what
> you need yard for during the build.
- Fixed
BuildRequires on rubygem(yard) is there because build fail without it, due to [1]

> - You don't need to use the "%defattr(-, root, root, -)" line, see [3]
- Yes, I'm aware of that, however i prefer to keep it there

> - Consider moving documentation into a subpackage (except of README.rdoc, which contains licensing info and therefore should be left in the main package).
- Documentation is too small to put it into subpackage, not worth of effort.

> - rpmbuild complains about History.rdoc and Manifest.txt listed twice. To solve
> it:
> ** the package shouldn't own the whole %{gemdir}/gems/%{gemname}-%{version}/
> directory (BTW you can use %{geminstdir} instead of it), but should rather own
> %dir %{geminstdir}
> ** when you do that, you will need to add every subdirectory and file in
> %{geminstdir} to %files, but you will be able to avoid the complaints about
> files listed twice
- Could not reproduce, works for me correctly (rpmlint-1.3.2).

[1] https://github.com/postmodern/hoe-yard/issues/1

Spec URL: http://hpejakle.fedorapeople.org/packages/rubygem-hoe-yard.spec
SRPM URL:
http://hpejakle.fedorapeople.org/packages/rubygem-hoe-yard-0.1.2-2.fc15.src.rpm

Comment 7 Bohuslav "Slavek" Kabrda 2011-11-18 06:55:50 UTC
(In reply to comment #6)
> > - You don't need to specify BuildRoot tag, see [2]
> - I know, as there is still open possibility to have this for EPEL in future, I
> keep it there.

Ah, I see. All right.

> > - You don't need to use the "%defattr(-, root, root, -)" line, see [3]
> - Yes, I'm aware of that, however i prefer to keep it there
> 

Any reasons for this? If it is not necessary, I recommend putting it away.

> > - Consider moving documentation into a subpackage (except of README.rdoc, which contains licensing info and therefore should be left in the main package).
> - Documentation is too small to put it into subpackage, not worth of effort.
> 

Ok, this is not a blocker, but next time, consider using gem2rpm from package rubygem-gem2rpm, which will generate a nice scaffold and you won't have to take care of this yourself.

> > - rpmbuild complains about History.rdoc and Manifest.txt listed twice. To solve
> > it:
> > ** the package shouldn't own the whole %{gemdir}/gems/%{gemname}-%{version}/
> > directory (BTW you can use %{geminstdir} instead of it), but should rather own
> > %dir %{geminstdir}
> > ** when you do that, you will need to add every subdirectory and file in
> > %{geminstdir} to %files, but you will be able to avoid the complaints about
> > files listed twice
> - Could not reproduce, works for me correctly (rpmlint-1.3.2).

Actually, you don't have to use rpmlint (and you are correct, it doesn't show this warning), it is written in build log, see my scratch build:

http://koji.fedoraproject.org/koji/getfile?taskID=3523114&name=build.log

Comment 8 Jan Klepek 2011-11-18 08:53:13 UTC
Ah, I see now. I did not checked build.log as far as it was builded correctly and in past, rpmbuild was not successful when such error happened, at least on my system. I will take a look.

regarding %defattr, I'm used to specify it this way, kind of historical reasons :). Does it bring any risk to user/infrastructure?

Comment 9 Vít Ondruch 2011-11-18 09:31:21 UTC
(In reply to comment #8)
> regarding %defattr, I'm used to specify it this way, kind of historical reasons
> :). Does it bring any risk to user/infrastructure?

This macro is not necessary in any supported RHEL nor Fedora if I am not mistaken. May be it is time to move forward?

The same with the BuildRoot. I would like to see there some conditional use, for older releases. Then it will be obvious that in some point in the future, the BuildRoot could be safely removed.

Comment 10 Bohuslav "Slavek" Kabrda 2012-08-03 10:56:16 UTC
So, any progress on this one?

Comment 11 Bohuslav "Slavek" Kabrda 2015-07-02 09:46:43 UTC
Since there seems to be no progress here, I'm closing this bug. Feel free to reopen if you wish to restart the review.


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