-
Notifications
You must be signed in to change notification settings - Fork 298
Update getting started guide to use spago #286
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
Conversation
I'd really rather get said cleanup done up front before merging in this case, as this guide is going to be people's very first experience with the language. Can you run through the guide as if you were a beginner following it, to make sure everything works the same way? |
cc @f-f would you mind taking a look at this? |
@@ -89,10 +89,11 @@ We will use a selection of these commands during this tutorial. | |||
|
|||
Start by pressing the Tab key to use the autocompletion feature. You will see a collection of names of functions from the Prelude which are available to use. | |||
|
|||
To see the type of one of these values, first import the appropriate module using the `import` command. `pulp init` configures `.purs-repl` to install `Prelude` automatically, so you won't have to do it yourself. |
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.
@f-f what do you think about having Spago create a .purs-repl
file including import Prelude
, like Pulp does, so that the repl "just works"?
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.
Having it create this file during spago init, I mean
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.
@hdgarrood yeah this is a great idea! I opened purescript/spago#555 to track this
Oh sorry, it looks like you already did this. Never mind 😅 what cleanup did you have in mind? |
@hdgarrood just overall structure and tone, less "clean up" and more "rethinking". But yes, just to be explicit, I did run through the entire guide in my console to make sure all commands work. |
@@ -16,48 +16,48 @@ The Purescript compiler (`purs`) can be installed with npm: | |||
|
|||
#### Setting up the Development Environment | |||
|
|||
PureScript's core libraries are configured to use the [Pulp](https://github.com/purescript-contrib/pulp) build tool, and packages are available in the [Bower registry](http://bower.io/search/?q=purescript-). | |||
PureScript's core libraries are configured to use the [Spago](https://github.com/spacchetti/spago) package manager and build tool. |
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.
If the move to purescript/spago
will happen quite soon we need to make sure to update this link (though fortunately GitHub will redirect automatically in the worst case).
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.
Yes, I was thinking about this as I pasted the link. That said, the update should be speedy (and, as you said, the redirect will work in the interim).
If everything was built successfully, and the tests ran without problems, then the last line should state "Tests OK". | ||
|
||
#### Installing Dependencies | ||
|
||
Dependencies can be installed using Bower. We will be using the `purescript-lists` library shortly, so install it now: | ||
Dependencies can be installed using Spago. We will be using the `purescript-lists` library shortly, so install it now: |
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.
Is it worth adding a line here that by convention / for historical reasons PureScript packages have a purescript-
prefix, but when installing them you just use the package name?
I can see it being a little odd to instruct someone to install purescript-lists
by typing spago install lists
without them knowing whether that's how all packages work or just this one.
Fortunately I believe Spago will install purescript-lists
by trimming the purescript-
prefix.
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.
Fortunately I believe Spago will install
purescript-lists
by trimming thepurescript-
prefix.
This is right. I think it's ok to reference the package with the prefix, as that's the name of the repo anyways? Though I have no strong opinion on this 🤷♂️
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.
In my opinion it's better to restructure this to work with Spago in the first pass and rethink how to introduce these tools to a beginner in a second pass. The "rethink" could easily take a lot more conversation to move forward than this simple tweak and I think it's valuable to have this happen sooner rather than later.
Yes, I suppose that's true. Ok, how about I'll merge this now, create an issue for remaining Bower updates (there are one or two other places in this repo we should probably update), and leave the rethink idea open for now; maybe we can wait and see if anything in here as it is now trips beginners up. |
Just fyi. Let me know whether your want to use any of the content in my
learning repo.
… |
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 left a small comment, but otherwise looks great! 👏
Co-Authored-By: Thomas Honeyman <admin@thomashoneyman.com>
Is there any more feedback preventing this from being merged? No rush, just wanted to make sure I've addressed everything. |
I don't have write access to this repository, but this looks good to me and I will merge the accompanying PR purescript/purescript.github.io#138 tonight if there are no objections (or no one merges it before me 🥇) |
Thanks! |
This updates the existing guide as-is to use
spago
rather thanpulp
andbower
. There's probably more clean up to be done here, but I wanted to keep this simple.