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

Dune & dune-release #67

Merged
merged 7 commits into from
Dec 17, 2018
Merged

Dune & dune-release #67

merged 7 commits into from
Dec 17, 2018

Conversation

Leonidas-from-XIV
Copy link
Member

Based on #66.

Copy link
Member

@pmetzger pmetzger left a comment

Choose a reason for hiding this comment

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

Thanks for this work! I'd like @mjambon to comment on the substantive changes before we merge the commits.

Your yojson.opam changes are going to conflict with the ones in the other PR, aren't they? Perhaps these should be separated?

.gitignore Outdated Show resolved Hide resolved
@mjambon
Copy link
Member

mjambon commented Dec 12, 2018

Looks good to me.

You should probably make the same changes in opam-repository if you haven't already. We should also replace my name/email by @NathanReb. This can be done later. See #68

Copy link
Member

@mjambon mjambon left a comment

Choose a reason for hiding this comment

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

Approving this. I'll let @pmetzger finish the discussion he started.

@Leonidas-from-XIV
Copy link
Member Author

Leonidas-from-XIV commented Dec 12, 2018 via email

yojson.opam Outdated Show resolved Hide resolved
yojson.opam Outdated Show resolved Hide resolved

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Thanks @avsm for the suggestion.
@Leonidas-from-XIV
Copy link
Member Author

I checked the build and it fails because the Mac builds can't find opam, so there is an unrelated problem in ocaml-ci-scripts.

When using dune-release it will generate the documentation automatically
and push it to the gh-pages branch which will deploy to this URL.
These had old instructions that weren't correct in the first place and
scripts that didn't work.
Copy link
Collaborator

@NathanReb NathanReb left a comment

Choose a reason for hiding this comment

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

Besides that it looks great and will definitely make further work on yojson easier, thanks for your time and effort!

examples/dune Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
yojson.opam Outdated Show resolved Hide resolved
@NathanReb
Copy link
Collaborator

Yeah don't worry about the CI, I'll see what I can do about that!

Makefile Outdated Show resolved Hide resolved
@NathanReb NathanReb merged commit c3e413a into ocaml-community:master Dec 17, 2018
NathanReb pushed a commit to NathanReb/opam-repository that referenced this pull request Dec 27, 2018
CHANGES:

### Changes

- Use dune as a build system (ocaml-community/yojson#67, @Leonidas-from-XIV)
- reraise exceptions in `finish_string` instead of silencing them by raising a `Failure _`
- raise finalizer exceptions in `from_channel` and `from_lexbuf` readers

### Fixes

- Fix a race condition in builds (ocaml-community/yojson#57, @avsm)
@Leonidas-from-XIV Leonidas-from-XIV deleted the dune branch September 3, 2019 15:55
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.

None yet

5 participants