[Bug 1833473] Review Request: ocaml-ppxlib - Base library and tools for ppx rewriters

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

 



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



--- Comment #7 from Jerry James <loganjerry@xxxxxxxxx> ---
Thank you for the review!

(In reply to Richard W.M. Jones from comment #3)
> Manual inspection of the spec file shows that it looks sensible.  Literally
> the only thing I could nitpick is that it would be nice to know if the two
> patches are upstream/going upstream/never upstream.

I would like to know that myself.  Well, upstream is aware of both issues and
will hopefully take action at some point.

(In reply to Richard W.M. Jones from comment #5)
> rjones: f-r says the package doesn't install, but I was able
> to install it just fine locally so I think we can ignore this.

Hmmm, the same thing happened when I did the review for you.  I just installed
manually and fixed up the report.  Apparently fedora-review is having issues
with installing.  I'll try to squeeze out some time to look into that.

> rjones: licensecheck is all over the place here, but most files lack
> any license information in the file.  It seems as if upstream intend
> this to be some kind of MIT license.  It would be good to encourage
> upstream to add a license to every file.

Okay, I'll talk to upstream about it.

> rjones: licensecheck is wrong here.  I manually checked the license
> and it is correct.

licensecheck always calls MIT-licensed files "Expat".  They're basically the
same license, so I guess that's okay, but it can be confusing.

> Ocaml:
> [x]: This should never happen
> 
> rjones: ?

That seems to come from /usr/share/fedora-review/plugins/ocaml.py, which says:

class OcamlCheckStaticLibs(OcamlCheckBase):
    """ Disable static lib checking, ocaml has an exception. """

    def __init__(self, checks):
        OcamlCheckBase.__init__(self, checks)
        self.automatic = True
        self.text = "This should never happen"
        self.deprecates.append("CheckStaticLibs")

    def run_on_applicable(self):
        self.set_passed(self.PASS)

I don't know "This should never happen" winds up in the report, though.

> rjones: I built it using rpmbuild.  It doesn't build in _my_ mock but
> it seems to be because my local mock is broken in some way.

FWIW, it builds fine in mock on my system.


-- 
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
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux