https://bugzilla.redhat.com/show_bug.cgi?id=1862624
Bug ID: 1862624 Summary: Review Request: ocaml-octavius - Ocamldoc comment syntax parser Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: loganjerry@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://jjames.fedorapeople.org/ocaml-octavius/ocaml-octavius.spec SRPM URL: https://jjames.fedorapeople.org/ocaml-octavius/ocaml-octavius-1.2.2-1.fc33.s... Fedora Account System Username: jjames Description: Octavius is an OCaml library to parse the ocamldoc comment syntax.
https://bugzilla.redhat.com/show_bug.cgi?id=1862624
Jerry James loganjerry@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |dan.cermak@cgc-instruments. | |com, rjones@redhat.com Doc Type|--- |If docs needed, set a value
https://bugzilla.redhat.com/show_bug.cgi?id=1862624
Jerry James loganjerry@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |1862625
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1862625 [Bug 1862625] Review Request: ocaml-ppx-js-style - Code style checker for Jane Street OCaml packages
https://bugzilla.redhat.com/show_bug.cgi?id=1862624
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |decathorpe@gmail.com Assignee|nobody@fedoraproject.org |decathorpe@gmail.com Flags| |fedora-review?
--- Comment #1 from Fabio Valentini decathorpe@gmail.com --- Taking this one.
https://bugzilla.redhat.com/show_bug.cgi?id=1862624
--- Comment #2 from Fabio Valentini decathorpe@gmail.com --- Same as in one of the other OCaml packages, the Provides seem to be duplicated between the main and -devel package.
There also seem to be minor isses with the executable files, as shown by rpmlint:
ocaml-octavius.x86_64: W: ldd-failed /usr/bin/octavius ocaml-octavius.x86_64: W: ldd-failed /usr/lib64/ocaml/octavius/octavius.cmxs ocaml-octavius.x86_64: E: non-standard-executable-perm /usr/bin/octavius 555 ocaml-octavius.x86_64: E: non-standard-executable-perm /usr/lib64/ocaml/octavius/octavius.cmxs 555
Not sure if ldd is supposed to fail on OCaml binaries, but I don't think so :-) And the executable permission should be 755, matching other files in /usr/bin.
Other than that, the package looks fine.
https://bugzilla.redhat.com/show_bug.cgi?id=1862624
--- Comment #3 from Jerry James loganjerry@gmail.com --- Thanks for the review, Fabio. That's weird. Dune usually sets the correct permissions on files. I don't know why it failed this time, but I have fixed them. New URLs:
Spec URL: https://jjames.fedorapeople.org/ocaml-octavius/ocaml-octavius.spec SRPM URL: https://jjames.fedorapeople.org/ocaml-octavius/ocaml-octavius-1.2.2-2.fc33.s...
https://bugzilla.redhat.com/show_bug.cgi?id=1862624
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST Flags|fedora-review? |fedora-review+
--- Comment #4 from Fabio Valentini decathorpe@gmail.com --- Well, I'm just hoping the 555 / 755 permission issues aren't too bad (either my PC is haunted or the glibc 2.33 dev snapshot is broken).
Package looks good now anyway.
- package is named correctly - ISC license is correct - latest version is packaged - follows OCaml packaging guidelines
It might be good to compare builds in mock and in koji, to check whether binaries actually get chmodded 555 or 755 ...
Package is APPROVED.
https://bugzilla.redhat.com/show_bug.cgi?id=1862624
--- Comment #5 from Jerry James loganjerry@gmail.com --- (In reply to Fabio Valentini from comment #4)
It might be good to compare builds in mock and in koji, to check whether binaries actually get chmodded 555 or 755 ...
Yes, I'm going to do one build then wait to see the results in koji before I do any more.
Package is APPROVED.
Thank you, Fabio! I appreciate the review.
https://bugzilla.redhat.com/show_bug.cgi?id=1862624
--- Comment #6 from Gwyn Ciesla gwync@protonmail.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/ocaml-octavius
https://bugzilla.redhat.com/show_bug.cgi?id=1862624
Jerry James loganjerry@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |CLOSED Resolution|--- |NEXTRELEASE Last Closed| |2020-08-14 21:30:54
--- Comment #7 from Jerry James loganjerry@gmail.com --- Built in F33 and Rawhide.
package-review@lists.fedoraproject.org