Opened 5 years ago

Closed 5 years ago

#18833 closed enhancement (fixed)

Clean up cliquer library interface

Reported by: jdemeyer Owned by:
Priority: minor Milestone: sage-6.8
Component: cython Keywords:
Cc: jpflori, ncohen Merged in:
Authors: Jeroen Demeyer Reviewers: Nathann Cohen, Jean-Pierre Flori
Report Upstream: N/A Work issues:
Branch: d26a000 (Commits) Commit: d26a00081cbda0a2992740208420e1a84f67a07d
Dependencies: Stopgaps:

Description

There is really no need to add .h files just to compile extra C code.

Change History (8)

comment:1 Changed 5 years ago by jdemeyer

  • Branch set to u/jdemeyer/remove_cl_h

comment:2 Changed 5 years ago by jdemeyer

  • Commit set to d26a00081cbda0a2992740208420e1a84f67a07d
  • Status changed from new to needs_review
  • Summary changed from Remove cl.h to Clean up cliquer library interface

New commits:

d26a000Remove cl.h

comment:3 Changed 5 years ago by jdemeyer

  • Cc jpflori added

comment:4 Changed 5 years ago by ncohen

  • Cc ncohen added

comment:5 follow-up: Changed 5 years ago by jpflori

The pro I see for this is:

  • you have to put the function signature in a pxd/pyx file anyway so the h file is superfluous.

Maybe a con:

  • the pro above is not that strong.

I don't have any strong opinion on this and the removal looks ok. Maybe Nathann has one.

comment:6 in reply to: ↑ 5 Changed 5 years ago by ncohen

I don't have any strong opinion on this and the removal looks ok. Maybe Nathann has one.

Same here. It looks cleaner, and I like a priori any patch that mostly removes code.

Nathann

comment:7 Changed 5 years ago by jpflori

  • Reviewers set to Nathann Cohen, Jean-Pierre Flori
  • Status changed from needs_review to positive_review

Ok then LGTM.

comment:8 Changed 5 years ago by vbraun

  • Branch changed from u/jdemeyer/remove_cl_h to d26a00081cbda0a2992740208420e1a84f67a07d
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.