Updated the waspc/README.md and run script.

[Martinsos]

Fixes #2584 and more.

New contributors were getting stuck on building ts-packages, as waspc/README.md was confusing regarding that (weren't telling them to build them before trying to run the project for the first time). Inspired by that, I added a ./run build:all command to run script and also edited waspc/README.md to instruct running that command early enough for the rest of the instructions to work.

I also updated all instrunctions in waspc/README.md to rely on ./run script and not cabal directly, since that way we avoid duplication of the logic (both in README and in run script) on how to run stuff, and instead we have ./run script as the source of truth.

While at it, I went a bit further and generally improved waspc/README.md in some other places where it seemed obvious.

I also did some small improvements to the run script to make it more presentable (coloring, truncation of lines, some spacing between groups of commands, ...).

[Martinsos]

I removed this because this section is now about what Wasp itself does, but about what technical details of how waspc is implemented.
There is actually another section more below that again repeats this information, so one more reason to remove it.

[Martinsos]

We were using just "NOTE" and "TIP", I switched that to these Github specific callouts that look nice.

[Martinsos]

This is new thing I added, how to figure out versions, since I figured out I actually don't know how to figure them out hah.

[Martinsos]

Now we tell them here to run this new command. So they don't have to know too much -> it just means they are building the whole project.

[Martinsos]

This was old info, those two got merged recently but we forgot to update this so I did it here.

[Martinsos]

I moved this below, this is too niche detail to be mentioned here.

[Martinsos]

Outdated, I updated it.

[Martinsos]

I don't explain cabal anymore in this README. If they want to learn cabal, they can read cabal docs, and if they want to know how to run the proejct, there is run script. No need to repeat oureslves or repeat docs that are not ours.

[FranjoMindek]

The less they have to learn to start working the better, I like it

[Martinsos]

This information still exists, but it got moved much sooner in this doc, in "Basics" where we introduce run script. I also shortened it a bit, without loosing anything I believe.

[Martinsos]

We are not really doing this so I removed it.

[Martinsos]

This I just moved from up there in the doc to here, I didn't change it.

[Martinsos]

This image was used in only one place in the code, in waspc/README.md, and it was outdated and repeating what is already explained in top level README.md, so I removed it.

[FranjoMindek]

I definitely like pictures with explanations. Might be good idea to reintroduce them for some parts one day.

[Martinsos]

I agree! But I skipped explaining the whole concept in this case.

[Martinsos]

I reorganized this a bit so now we have build(hs), build:all, and build:packages. Check the actual commands lower in the file.

[Martinsos]

Text is now truncated at 80 lines. This improved readability.

[Martinsos]

Renamed this to build:packages.

[Martinsos]

No we introduce run script much earlier, at the very start. Which makes sense, since the rest of this file will be mentioning it a lot.

[FranjoMindek]

I like it

[Martinsos]

I split the setup into 1. setting up Haskell stuff and 2. setting up npm stuff. I also simplified it a bit. I think this makes it very easy to understand and do now.

[Martinsos]

For reviewing this file, I would also advise checking it out directly so that you get proper markdown rendered/preview and can read it in one piece nicely.

[FranjoMindek]

I like the total change of how it reads. Some grammar stuff and suggestions.

I've also noticed stuff I want to fix on current version of docs but not in the diffs.

e.g.

• under Important directories (in waspc/) title, the first bullet points don't use inline code for directories
• often lack of inline code for code related names of stuff e.g. in codebase overview

How do we want to address such stuff?

[FranjoMindek]

I like it

[FranjoMindek]

The less they have to learn to start working the better, I like it

[FranjoMindek]

I definitely like pictures with explanations. Might be good idea to reintroduce them for some parts one day.

[Martinsos]

Thanks @FranjoMindek for all the oxford commas :D (among the other stuff), please take another look!

[FranjoMindek]

Just a tiny bit of more changes.
No more oxford commas 😆 .

[FranjoMindek]

Checked my config and its:

export PATH="/opt/homebrew/opt/llvm@13/bin:$PATH"

Both file paths exist so I researched:
https://stackoverflow.com/questions/35337601/why-is-there-a-usr-local-opt-directory-created-by-homebrew-and-should-i-use-it

I think the brew's post installation path recommendation is more correct here, as it's a stable, version-agnostic path.

Redoing this part after we update the toolchain also sounds good. 👍

[FranjoMindek]

Looks good to me...

[infomiho]

Some of the details are missing and the wrun test:headless won't work after you merge the main branch into yours

[infomiho]

(that you previously built with ./run build:all)!

The command will build the wasp-cli if not built, let's maybe drop this bit not to confuse people that they might need to run build:all every time.

[Martinsos]

The difference between build:all and other builds is explained just below in "typical development workflow" so I think there is no danger of them using it wrong, and on the other hand I think it is avluable here as it establishes connection between this executable and the build above, so I tihnk it is useful to leave this!

It will build wasp-cli if not built, but it won't build the ts-packages. If they indeed don't get to understanding the differene between different build coomands, I would rather have them use build:all each time than not being able to get stuff working because they are not building ts packages.

[FranjoMindek]

This might lead people into believing build:all is required every time. I would like to avoid that. So I agree with @infomiho here. Is explicit connection to ./run build:all needed here since we can assume the contributors know what "build" and "run" are?

[Martinsos]

@FranjoMindek I think I explained this above in the message: Why I don't think this is an issue + why I do think it is needed. Is there something there you don't agree with?

[FranjoMindek]

I just think that we don't need to establish the connection between build and executable. That was my intention. But the more I think about it maybe for the first encounter it's good that it's there.

[Martinsos]

I just think that we don't need to establish the connection between build and executable. That was my intention. But the more I think about it maybe for the first encounter it's good that it's there.

If I am the only one thinking it is useful I will remove it, might be I am in my bubble here hah, but I thought it helps to establish the connect as we never say what is being built in the step above, and I don't want to list all that is being built there beacuse then we needt o mainta that list, so I thought this is a happy mid solution.
And later there are details on what typical dev process looks like.

I won't push it further though, I said all I had -> if you two would still rather I just remove that part about build:all, I will.

[infomiho]

I've noticed that test:headless doesn't work if you are not in the waspc folder:

wrun test:headless
Running: cd examples/todoApp && npm install && npx playwright test --config headless-tests 

/Users/ilakovac/dev/wasp/waspc/run: line 57: cd: examples/todoApp: No such file or directory

Also, we are missing the bit where we install the wasp-app-runner (which is merged into main) and I think this kind of command would work:

TEST_HEADLESS_CMD="cd $PROJECT_ROOT/../wasp-app-runner && \
npm install && \
npm run install:global && \
cd $PROJECT_ROOT/examples/todoApp && \
npm install && \
DEBUG=pw:webserver npx playwright test --config headless-tests"

(Split it into multiple lines for your easier reading)

[Martinsos]

Good catch! However, it is out of scope of this PR: I didn't add ./run test:headless command here. Meaning that this is also like this on main. So I wouldn't solve that here, we should solve that with a separate PR. Can you please create an issue (or maybe PR immediately if it is simple enough) for it since you know this stuff better than me?

[infomiho]

Created an issue to solve this (I'll fix it today, it's a simple change) #2673

[infomiho]

Here's the PR #2674

[Martinsos]

I did another iteration!
@infomiho I believe I fixed everything you suggested except for that ./run test:headless comment -> but that one, if I am correct, has nothing to do with this PR and is also broken on the main branch, and was probably supposed to be implemented in the PR that implemented the app runner. Now it should probably be implemented via separate PR as a fix.

[FranjoMindek]

Noticed this during a re-read.
I stayed out of run commands changes since I believe others handle that part much better.

[Martinsos]

@FranjoMindek I accepted 2 suggestions and answered one comment, take a look again.

[FranjoMindek]

We only need to fix the title link now.
I've approved the PR since it's a small thing.

[Martinsos]

We only need to fix the title link now. I've approved the PR since it's a small thing.

Nice, fixed! I missed previously that you wrote you are not usre about the link, my bad.

[infomiho]

good to go