[Bug 894482] Review Request: rubygem-openshift-origin-auth-kerberos - OpenShift plugin for kerberos auth service

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=894482

--- Comment #15 from Vít Ondruch <vondruch@xxxxxxxxxx> ---
* No test suite
  - I see that the test suite is not executed nor is any available upstream.
    You should definitely do something about it, although it is not blocker.
    I just need to do this remark, sorry ;)

* Macroize /etc
  - For /etc, we have %{_sysconfdir} macro [1]
  - Looking at rubygem-openshift-origin-common, there is the same issue
  - Have you ever considered creating some package, such as in ruby we have
    rubygems-devel, which would carry definition of some common macros used by
    OS? For example the /etc/openshift and /etc/openshift/plugins.d paths looks
    like good candidates for such package.

* -doc subpackage should be noarch
  - Documentation is typically arch independent, isn't it?

* -doc subpackage does not own directories
  - The -doc subpackage depends on the main package typically, in that case,
    the package requires rubygems and everything is ok. However, if you do not
    require the main package, you should take care of ownership of the whole
    path to the documentation, which I don't believe you did. You own just the
    doc dir under rubygems directory, not the rest.
  - To solve that, I would suggest to require the main package as we usually
do.

* -doc subpackage without group
  - The group is not mandatory anymore, but for documentation, it comes natural
    to me, that the group should be specified there, but this is not blocker. 

* Use %gem_install macro
  - Is there any reason, why you are not using %gem_install macro? Is that due
    to RHEL? If yes, I'd love to see it conditionalized anyway.

* Move Gemfile into -doc subpackage
  - This file is not needed for runtime, so I would suggest to move it into
    -doc subpackage (or alternatively just exclude it).

* Prefix %{gem_instdir} with %dir
  - I always suggest to do %dir %{gem_instdir} so you have better control what
    goes into the package
  - Currently, I would say (although I did not build the package) that you have
    duplicated LICENSE COPYRIGHT Gemfile files. One version goes into
    /usr/share/doc and their second version redisdes in %{gem_instdir}, but
    I might be mistaken

[1] https://fedoraproject.org/wiki/Packaging:RPMMacros

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=2iNjTvyU3v&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]