Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement hub in Ocaml #253

Closed
wants to merge 3 commits into from
Closed

Conversation

tmcgilchrist
Copy link
Member

No description provided.

Copy link
Member

@avsm avsm left a comment

Choose a reason for hiding this comment

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

Looks good to me sans some minor comments. Needs a CHANGES entry as well.

hub/hub.ml Outdated Show resolved Hide resolved
hub/dune Outdated Show resolved Hide resolved
@@ -1,32 +0,0 @@
open Printf
Copy link
Member

Choose a reason for hiding this comment

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

intentionally deleting the releases binary here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes that is deliberate for 2 reasons:

  1. The hub command can test the same code minor formatting differences aside
    eg dune exec -- hub/hub.exe release --owner mirage --repo ocaml-github
  2. This binary wasn't listed in the dune file so it wasn't being compiled.

In future I'd like to have lib_test executables bundled into the hub command with some mdx tests wrapping them to provide some basic assurances.

@avsm
Copy link
Member

avsm commented Jul 7, 2021

This needs to add a yaml dependency to the opam file.

@tmcgilchrist
Copy link
Member Author

I'm unlikely to work on this anytime soon, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants