Skip to content

Yet more sexps#250

Merged
hannesm merged 4 commits into
masterfrom
yet-more-sexps
May 15, 2015
Merged

Yet more sexps#250
hannesm merged 4 commits into
masterfrom
yet-more-sexps

Conversation

@pqwy
Copy link
Copy Markdown
Contributor

@pqwy pqwy commented Feb 8, 2015

Gets rid of the local cstruct pretty-printing code, delegates to the more compact native version.

Merging should probably wait for mirage/ocaml-cstruct#44, but this is what Piñata runs right now.

Comment thread lib/state.ml
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this is an error, why not just expose only sexp_of below?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One, to get downstream types to use with sexp and avoid the boilerplate of writing [one side of] it by hand. Two, because this is expected to be fixed eventually (it's in a (* Sexplib stubs *) ... (* *** *) comment block), and it's nice to have the API, even if sketched out.

Note that all these types should be invisible to the outside. This is all basically for tracing/debugging.

@hannesm
Copy link
Copy Markdown
Member

hannesm commented Apr 28, 2015

now that cstruct-1.6.0 is out, could you rebase? then we could merge and be happy..

@avsm
Copy link
Copy Markdown
Contributor

avsm commented Apr 28, 2015

And update the opam file constraint at same time as rebase

On 28 Apr 2015, at 19:56, Hannes Mehnert notifications@github.com wrote:

now that cstruct-1.6.0 is out, could you rebase? then we could merge and be happy..


Reply to this email directly or view it on GitHub.

@pqwy pqwy force-pushed the yet-more-sexps branch from aabb0b9 to 08d35f4 Compare May 1, 2015 15:31
@pqwy
Copy link
Copy Markdown
Contributor Author

pqwy commented May 1, 2015

Rebased.

But once merged, it will require the unreleased mirage-xen 2.3.2 with support for cstruct 1.6.0.

ping @avsm

@pqwy pqwy force-pushed the yet-more-sexps branch from ea8b265 to d4c3775 Compare May 15, 2015 14:55
hannesm added a commit that referenced this pull request May 15, 2015
@hannesm hannesm merged commit ee520c5 into master May 15, 2015
@hannesm hannesm deleted the yet-more-sexps branch May 15, 2015 15:29
@hannesm
Copy link
Copy Markdown
Member

hannesm commented May 15, 2015

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants