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

Use Diskuv OCaml as the current Windows option #539

Merged
merged 15 commits into from Sep 24, 2022
Merged

Conversation

jonahbeckford
Copy link
Contributor

No description provided.

@jonahbeckford
Copy link
Contributor Author

Note: The failing CI Build has nothing to do with the PR!

@Leonidas-from-XIV
Copy link
Contributor

Since #543 was merged, the CI is fixed so if you rebase on main the PR it should finish successfully :)

Copy link
Collaborator

@tmattio tmattio left a comment

Choose a reason for hiding this comment

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

Thanks a lot Jonah!

I see that you updated the Up and Running documentation. All the changes look good (my suggestions on the recommended Windows method aside, more on that below), but we recently rewrote that page in anticipation to the release of OCaml 5: https://staging.ocaml.org/docs/up-and-running. I don't mind merging changes to the Up and Running documentation now, but would you like to also port your changes to the staging branch?

Re. the new Windows documentation, I'm not sure about making Diskuv the default installation method.

Granted, it is simpler to install than Cygwin + fdopen's repository, but I think the recommended method should support the latest compiler version, especially with the upcoming OCaml 5 release (it'll be a bit awkward when we advertise OCaml 5 that our official documentation recommends not to use it).

It'd be great to point to Diskuv as a Windows solution, but perhaps as an alternative if one is willing to deviate from the official compiler release cycle?

The decision process, IMO, would look like this:

  • When you want to develop applications, use Cygwin + fdopen's repo
  • When you want to develop applications, care about stability, and don't care about the compiler version, use Diskuv.
  • When you only want to run applications, use WSL or Docker

What do you think?

data/tutorials/gs_03_ocaml_on_windows.md Outdated Show resolved Hide resolved
data/tutorials/gs_03_ocaml_on_windows.md Outdated Show resolved Hide resolved
@jonahbeckford
Copy link
Contributor Author

jonahbeckford commented Aug 9, 2022

It'd be great to point to Diskuv as a Windows solution, but perhaps as an alternative if one is willing to deviate from the official compiler release cycle?

The decision process, IMO, would look like this:

  • When you want to develop applications, use Cygwin + fdopen's repo
  • When you want to develop applications, care about stability, and don't care about the compiler version, use Diskuv.
  • When you only want to run applications, use WSL or Docker

What do you think?

Agree. And I'll copy-edit the rest so your good point about not tracking the official compiler release cycle is emphasized.

Edit: I missed over the fdopen repository part of Cygwin + fdopen; I mixed that up with Cygwin + central Opam with appropriate available: [ os != win32 ] flags. But it doesn't seem responsible to recommend developing an application with a repository where, on the same page, we say (with added italics):

For a long time this has been maintained by @fdopen ... As of August 2021, the repository will no longer be updated

IMHO, recommending something is equivalent to supporting the user when they file a ticket. From what I understand the only place for a Windows user today to get a response for a Windows Opam package related issue is to file an issue with Diskuv's distribution. User support has been my implicit dividing line between "primary" recommendations and "alternatives". There could be some other important dividing lines, and I think you are alluding to OCaml 5 support being one dividing line (which makes sense).

Perhaps we need multiple dividing lines like the Tier N labeling of the OCaml compiler: Tier 1 ("Actively Maintained"; Ticket Supported + Latest Compilers), Tier 2 ("Maintained when possible"; Ticket Supported) and Tier 3 (User Supported). Tier 1 would be what you are calling Opam 2.2. Tier 2 would be Diskuv. Tier 3 would be fdopen, Docker and WSL2.

Does that grouping work better?

@jonahbeckford
Copy link
Contributor Author

I don't mind merging changes to the Up and Running documentation now, but would you like to also port your changes to the staging branch?

Yes, I'll merge the changes to staging branch as well.

@jonahbeckford
Copy link
Contributor Author

Ported to staging branch with same content but re-organized in a way that de-emphasizes Diskuv OCaml (platform installer + Opam 2.2 should be used instead). #554

Incorporated feedback.

Copy link
Collaborator

@christinerose christinerose left a comment

Choose a reason for hiding this comment

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

Mostly line editing for clarity, grammar, consistency, and syntax. There will be some conflicts with the recently merged PR, as I had also gone through this, and I think I incorporated some of these suggestions in explaining the shell in Up and Running.

data/tutorials/gs_00_up_and_running.md Outdated Show resolved Hide resolved
data/tutorials/gs_00_up_and_running.md Outdated Show resolved Hide resolved
data/tutorials/gs_00_up_and_running.md Show resolved Hide resolved
data/tutorials/gs_00_up_and_running.md Outdated Show resolved Hide resolved
data/tutorials/gs_00_up_and_running.md Outdated Show resolved Hide resolved
data/tutorials/gs_03_ocaml_on_windows.md Outdated Show resolved Hide resolved
data/tutorials/gs_03_ocaml_on_windows.md Outdated Show resolved Hide resolved
data/tutorials/gs_03_ocaml_on_windows.md Outdated Show resolved Hide resolved
data/tutorials/gs_03_ocaml_on_windows.md Outdated Show resolved Hide resolved
data/tutorials/gs_03_ocaml_on_windows.md Outdated Show resolved Hide resolved
@tmattio
Copy link
Collaborator

tmattio commented Sep 2, 2022

@jonahbeckford seeing the number of PRs made on the Up and Running tutorial, we decided to merge the new version that was on staging immediately to avoid having to rebase constantly. I can rebase your current PR on top of main if you want. A lot of changes in the new version are actually extracted from your PR, with some edits from Christine.

@jonahbeckford
Copy link
Contributor Author

I can/will rebase.

patricoferris pushed a commit to patricoferris/v3.ocaml.org-server that referenced this pull request Sep 2, 2022
Jonah Beckford and others added 9 commits September 2, 2022 09:55
Co-authored-by: Christine Rose <christinerose@users.noreply.github.com>
Co-authored-by: Christine Rose <christinerose@users.noreply.github.com>
Co-authored-by: Christine Rose <christinerose@users.noreply.github.com>
Co-authored-by: Christine Rose <christinerose@users.noreply.github.com>
Co-authored-by: Christine Rose <christinerose@users.noreply.github.com>
Co-authored-by: Christine Rose <christinerose@users.noreply.github.com>
Co-authored-by: Christine Rose <christinerose@users.noreply.github.com>
@jonahbeckford
Copy link
Contributor Author

I rebased. I think I got all the suggestions merged, but now there is also a bit of new content.

jonahbeckford and others added 3 commits September 5, 2022 08:31
Co-authored-by: Christine Rose <christinerose@users.noreply.github.com>
Co-authored-by: Christine Rose <christinerose@users.noreply.github.com>
Co-authored-by: Christine Rose <christinerose@users.noreply.github.com>
Copy link
Contributor Author

@jonahbeckford jonahbeckford left a comment

Choose a reason for hiding this comment

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

Update to Diskuv OCaml v1.0.1

data/tutorials/gs_00_up_and_running.md Outdated Show resolved Hide resolved
@tmattio
Copy link
Collaborator

tmattio commented Sep 6, 2022

Thanks for rebasing @jonahbeckford! I had another review pass and it looks really good to me.

@christinerose @panglesd do you want to have a look at the version after the rebase? Happy to merge when it looks good to you too.

data/tutorials/gs_00_up_and_running.md Outdated Show resolved Hide resolved
data/tutorials/gs_00_up_and_running.md Outdated Show resolved Hide resolved
data/tutorials/gs_00_up_and_running.md Outdated Show resolved Hide resolved
data/tutorials/gs_00_up_and_running.md Outdated Show resolved Hide resolved
data/tutorials/gs_00_up_and_running.md Outdated Show resolved Hide resolved
data/tutorials/gs_00_up_and_running.md Outdated Show resolved Hide resolved
data/tutorials/gs_03_ocaml_on_windows.md Outdated Show resolved Hide resolved

Our guidance is when you want:

* **only to run, not develop, applications**, use [Docker](#docker-images) or [WSL2](#wsl2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are so many commas in this sentence.
Perhaps we can reword it?
Like:
only to run (not develop) applications, use Docker or WSL2

data/tutorials/gs_03_ocaml_on_windows.md Outdated Show resolved Hide resolved
data/tutorials/gs_03_ocaml_on_windows.md Outdated Show resolved Hide resolved
tmattio and others added 2 commits September 24, 2022 10:07
Co-authored-by: Christine Rose <christinerose@users.noreply.github.com>
Co-authored-by: Christine Rose <christinerose@users.noreply.github.com>
@tmattio tmattio merged commit e266ecf into ocaml:main Sep 24, 2022
@tmattio
Copy link
Collaborator

tmattio commented Sep 24, 2022

Thanks a lot @jonahbeckford, it's a massive improvement over the previous Windows setup documentation! 🎉

@cuihtlauac cuihtlauac mentioned this pull request Oct 20, 2022
sabine pushed a commit that referenced this pull request Oct 21, 2022
Co-authored-by: Christine Rose <christinerose@users.noreply.github.com>
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

4 participants