-
Notifications
You must be signed in to change notification settings - Fork 345
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
Up and running with OCaml #1165
Conversation
Oh, I should say that this is intended to be good enough, before merging, to be the page loaded when someone clicks `Install OCaml' on the front page. So please read with that in mind. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I left a couple minor comments.
sh <(curl -sL https://raw.githubusercontent.com/ocaml/opam/master/shell/install.sh) | ||
``` | ||
|
||
(If this method does not suit, please see [How to install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a minor point, but "does not suit" reads slightly strange to me (I would expect "suit you", but rather use another wording); maybe it's more common in regional versions of English I'm less familiar with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it to "suit you", which is probably better. What I was getting at, I think, with the choice of verb, is that some people don't like to curl and execute random scripts from the internet.
(The disadvantage of being a native English speaker, is that I have no way of explaining why the verb suit doesn't always need an object. The closest I can think of is that "fit" doesn't either e.g. "If the last piece fits you have finished.")
|
||
##Editor support for OCaml | ||
|
||
**For Vim and Emacs**, install the merlin system using OPAM: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a link to the Merlin homepage would be nice (users can follow it if they want to know what kind of feature Merlin provides)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. (I also note that there are LSP implementations for vim and emacs on the web, but I assume plain merlin is still the easiest way for these editors...)
the installation procedure will print instructions on how to link merlin with | ||
your editor. | ||
|
||
For **Visual Studio Code**, the OCaml language server can by installed by OPAM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe something like "For Visual Studio Code and other LSP-supporting editors"? I have the impression that this description applies to most editors that recognize themselves as IDEs these days (of course the specific plugin will vary, and in some cases may not exist), and I would regret that people using another IDE would feel left out when the answer is most likely to be the same. (But then maybe those people don't really exist for OCaml anymore, and it's better to only talk about things we know for sure we support well.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
|
||
*Table of contents* | ||
|
||
#Up and Running with OCaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comment. The rendering of your headings by Github is off (see https://github.com/johnwhitington/ocaml.org/blob/up-and-running/site/learn/tutorials/up_and_running.md), I think you should use a space before the title: ## Up
rather than ##Up
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Thanks.
Ok, I'm finished for now. Let's get this merged, if everyone agrees, and we can see how beginners cope with it. |
``` | ||
|
||
(If this method does not suit you, please see [How to install | ||
OPAM](opam.ocaml.org/doc/install.html)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One issue that I have with that formulation is that the curl based installation would by my option of last resort on a linux distribution with outdated version of opam. Maybe it would make sense to inverse the two options: first start with the system-managed option, before moving to the user-managed script option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this section, I copied the instructions from the official guide: https://opam.ocaml.org/doc/Install.html
I can add a mention that any existing version should be removed first, if that would suffice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have the same issue with the opam page. In particular, the fact that the recommended installation path appears only in the third section is problematic. The recommended path should appear first to let readers skip the non-recommended options if they can. And removing any existing version is not the issue at hand?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I did not spot the phrase "This is generally the recommended way, when available and up-to-date" on the OPAM page. I will have a think about this.
``` | ||
|
||
The installation procedure will print instructions on how to link Merlin with | ||
your editor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be worth it to mention opam user-setup
rather than refer the reader to some unspecified documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation is what opam install merlin
prints out - I don't paste it into the tutorial, but I'm not sure how it's unspecified?
Is user-setup
up to date? It looks not to have been updated in a while. Is it widely used? Do the merlin developers recommend it? I'm looking for consensus when writing this tutorial, above all.
Do you suggest just mentioning it, or making it the default instruction?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reader cannot infer how involved is the remaining installation procedure with just a "Follow the printed instructions". And user-setup
, it is still one of the option cited by merlin post-install and is mentioned in the merlin quick setup section. Moreover, it has the advantage to be editor independent which means that we can give the reader the information that the quick setup can be as quick as
opam user-setup install
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. What's the Windows status?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to deal with this in a followup PR, as I suspect noone is entirely sure what the opam user-setup
status is. CCing @emillon who has been investigating the Windows support of Platform tools recently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opam user-setup seems to start on windows, but displays some error messages and tries to detect ocamltop, emacs, vim, gedit and sublime3. I don't think it does something windows specific at the moment.
I ran
I don't see how we can recommend it in this state... (Edit: merlin works fine on my computer otherwise...) |
Ok, I've finished today's changes. Ready for further review whenever. |
Fair point! (I hope you will report the issue on the appropriate development repository.) |
|
This looks like a nice proposal for a tutorial that would already improve the ocaml.org experience for beginners. (I may be useful to link to this tutorial from the Install page, but that could maybe be discussed independently?) I would be happy to see this discussed in its current, rather-final form, but it is not clear who would possibly make merge decisions or request changes. (cc @agarwal, @avsm, @Chris00; are there other people with commit rights that I should consider pinging, or an alias to use in this situation?) Note: (everyone cc-ed in this thread knows this, but to give more context) I'm part of the OCaml Foundation who funded this work from @johnwhitington. I'm not trying to influence people to merge it (I think it should be discussed on its own merits of course), only to influence people to give feedback and hopefully make decisions :-) |
Many thanks, this very close to mergeable. I've pushed a snapshot of this to the |
|
||
### For Linux and MacOS | ||
|
||
We will install OCaml using OPAM, the OCaml package manager. OPAM will also be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will install OCaml using OPAM, the OCaml package manager. OPAM will also be | |
We will install OCaml using opam, the OCaml package manager. opam will also be |
opam should be lower case
|
||
``` | ||
# Homebrew | ||
brew install gpatch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this gpatch
step necessary? Just brew install opam
should be sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took the MacOS instructions from here:
https://opam.ocaml.org/doc/Install.html
So if they are out of date, we need to fix them there too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have asked for an update here:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I have removed the instruction on the assumption your patch will be approved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR for the opam install page:
The OCaml toplevel, version 4.11.1 | ||
``` | ||
|
||
**For either Linux or MacOS** as an alternative, a binary distribution of OPAM is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
**For either Linux or MacOS** as an alternative, a binary distribution of OPAM is | |
**For either Linux or macOS** as an alternative, a binary distribution of opam is |
Protocol, the OCaml language server can by installed with OPAM: | ||
|
||
``` | ||
$ opam pin add ocaml-lsp-server https://github.com/ocaml/ocaml-lsp.git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pin is no longer necessary, as lsp has been released into opam
@johnwhitington with apologies for the delay, I've left some fairly minor comments. If you don't have time to address them, I can just fix them and merge in one go so this isn't further delayed. |
I have made the changes, with the exception of the This can be merged now, although I presume there will be a conflict between this and my other pull request (rearranging tutorials #1179 ), since this pull request adds an entry to the tutorial contents page. Perhaps it is best to merge #1179 first, then I can fix the conflicts in this one? |
This PR reflects three lessons learned when writing the new ocaml.org "Up and Running" guide ocaml/ocaml.org#1165 : 1) Having the binary distribution at the top of this page, when it is not the recommended installation mechanism for most users, is confusing. 2) Brew will shortly be updated Homebrew/homebrew-core#64301 to make opam depend on gpatch, thus removing this instruction from the OS X instructions. 3) OSX is now spelled macOS
If you could merge this to master, that will also pick up a working CI. |
I executed:
It seemed to do something, but nothing has changed on this page. Should it have done? |
You'll need to pull from the master branch of ocaml.org itself. Try:
There are ways to make the above shorter, but not worth the git risk/reward :-) |
Thanks. |
Thanks again! |
... and thanks @avsm for the review work and decision-making. It looks like the implicit answer to my question above is "Anil is going to find extra hours in the day to look at these PRs", which is great in the short term, but please let us know if there is something more scalable we could do in the medium term. (I'm not volunteering to become a reviewer for ocaml.org, but I could think of volunteering other people if that sounded useful.) |
I'll run it for the next six months at least to get the momentum going again: @patricoferris @kanishka-work and @ILeandersson have all offered to help with modernising the website so @agarwal and @GemmaG and I are mentoring them into it and working on making the site more contributor friendly. So please do keep the content updates coming :-) |
This PR reflects three lessons learned when writing the new ocaml.org "Up and Running" guide ocaml/ocaml.org#1165 : 1) Pending: having the binary distribution at the top of this page, when it is not the recommended installation mechanism for most users, is confusing. 2) Brew will shortly be updated Homebrew/homebrew-core#64301 to make opam depend on gpatch, thus removing this instruction from the OS X instructions. 3) OSX is now spelled macOS Co-authored-by: Etienne Millon <etienne.millon@gmail.com>
This PR reflects three lessons learned when writing the new ocaml.org "Up and Running" guide ocaml/ocaml.org#1165 : 1) Pending: having the binary distribution at the top of this page, when it is not the recommended installation mechanism for most users, is confusing. 2) Brew will shortly be updated Homebrew/homebrew-core#64301 to make opam depend on gpatch, thus removing this instruction from the OS X instructions. 3) OSX is now spelled macOS Co-authored-by: Etienne Millon <etienne.millon@gmail.com>
This is a first crack at my proposal from ocaml/ocaml.org#1137
Please let me have any comments in the next couple of weeks or so, and I will polish it up (and test all the instructions from scratch on all platforms) so it is suitable for merging.
The aim is to have an uncontroversial, simple, set of instructions for getting up and running with OCaml which works on all platforms.
I am interested both in comments about scope, and about the actual contents.
cc @gasche @avsm