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

(docs) Use DkML 2.1.0 #2249

Merged
merged 3 commits into from Mar 28, 2024
Merged

(docs) Use DkML 2.1.0 #2249

merged 3 commits into from Mar 28, 2024

Conversation

jonahbeckford
Copy link
Contributor

  • Correct DkML capitalization
  • Remove outdated name "Diskuv OCaml" and replace with DkML
  • Update WSL2 with 4.14.2
  • Replace usages of DkML installer with DkML distribution since winget is now the controlling installer on Windows
  • Remove opam env variations for DkML
  • Restore ppx_show example for DkML since it works (obviously without the earlier Dream example)

- Correct DkML capitalization
- Remove outdated name "Diskuv OCaml" and replace with DkML
- Update WSL2 with 4.14.2
- Replace usages of DkML installer with DkML distribution since winget is now the controlling installer on Windows
- Remove `opam env` variations for DkML
- Restore `ppx_show` example for DkML since it works
Copy link
Collaborator

@cuihtlauac cuihtlauac left a comment

Choose a reason for hiding this comment

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

Thanks @jonahbeckford. It's great to see this material updated.

I'm still confused about the compatibility status of the examples.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not clear to me.

  • Are the examples working with DkML 2.1.0?
  • Are the examples working with WSL?
  • Are the examples working with Cygwin?

We should be explicit about what is working or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested it with DkML 2.1.0 and it works. So, I removed the "OCaml DKML installer due to a dependency issue" DkML-specific comment.

This is a PR for DkML 2.1.0. I don't think I should be on the hook to test WSL2 and Cygwin. Hopefully ocaml.org uses MDX so the examples can be tested in CI. If you do have CI, it is straightforward to add DkML to CI.

Side-note: I did nothing to fix the issue because I was completely unaware of that comment. It is weird that is works, and that makes me highly suspicious of the DkML-specific comment. I suspect that the comment was really for Dream (next side-note). Hopefully we can file issues in the future if there really was a problem with a simple PPX.

Side-note 2: The earlier Dream example does not work with DkML. But we've already discussed in #1962 that Dream has a huge dependency cone and to come up with alternatives.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, that's clear, thanks a lot. Maybe we could say something like this:

Note: This example was successfully tested on Windows using DkML 2.1.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That sounds confusing to the user ... in particular, it begs the following questions:

  1. How do I tell what version I am on?
  2. Why doesn't every example tell me which versions, operating systems and distributions it was tested on?

It seems like a good addition as an HTML comment for ocaml.org maintainers. Is that what you meant?

EDIT: In the interests of time, I added it in with some language on how to check the version. Since I know of at least one user today who was messed up by not having this PR, if you have more minor suggestions can you just add them and submit? Thx.

data/tutorials/getting-started/1_00_install_OCaml.md Outdated Show resolved Hide resolved
jonahbeckford and others added 2 commits March 26, 2024 09:04
Co-authored-by: Cuihtlauac Alvarado <cuihtlauac@users.noreply.github.com>
Add suggestion from cuihtlauac
Copy link
Collaborator

@cuihtlauac cuihtlauac left a comment

Choose a reason for hiding this comment

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

Thanks @jonahbeckford, let's merge, you are right, this is needed by the community

@cuihtlauac cuihtlauac merged commit 3e4f950 into ocaml:main Mar 28, 2024
3 checks passed
@sabine sabine changed the title Use DkML 2.1.0 (docs) Use DkML 2.1.0 Apr 2, 2024
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

3 participants