[Bug 1310674] Review Request: rubygem-github-linguist - GitHub Language detection

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

 



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

Vít Ondruch <vondruch@xxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-review?              |fedora-review+



--- Comment #9 from Vít Ondruch <vondruch@xxxxxxxxxx> ---
First of all, thank you for enabling most parts of the test suite. Great job!

A few random notes:

* Source1 comments
  - I would suggest to convert the vim commentary into seds, that would make it
    more or less executable. And since I am at it, if you moved these into
    %check section, they would really be executable. Not a blocker however.

* The test result grep
  - You are checking just for failures, but I would suggest to check also for
    errors, e.g. currently to grep for "3 failures, 14 errors"
  - The comment could be more to the point, why there are still some failures.
  - Neither of this is blocker.

* Changelog entries
  - The changelog entries could be more descriptive probably ...

* rpmlint
  - rplint complains about one file:

    rubygem-github-linguist.noarch: E: script-without-shebang
      /usr/share/gems/gems/github-linguist-4.7.6/lib/linguist/languages.yml

* BR: npm
  - I don't think that you need the npm dependency. I just tested the build
    without NPM and it just passes.


Since these are just minor nits, I am going to APPROVE the package, but please
fix the issues prior import.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
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]