Update CONTRIBUTING.md

[MarkMcDaniels]

Added the explanation of what is being done with the commands from "Initial Setup (Docker)" before the new code. Duplicated this at the end of "int setup docker" as well.

I'm not sure if that's what you wanted, but I didn't want to change too much.

Closes #142

[allella]

@zach2825 are yarn prod and yarn dev still used in the HG app?

Mark is updating the notes in CONTRIBUTING.md and these commands aren't obvious for folks like me.

I'm assuming they are as defined in package.json.

What's the significance of of the hg: in front of the scripts in package.json, like with hg:install?

[allella]

@MarkMcDaniels I'm working to get answers on the questions above.

I do think we remove the part about php artisan migrate --seed being run by composer if we're always having people run composer install, and since it handles it.

We can wait for more clarification on the commands to run for dev versus production use cases. Also, we'll need @oliviasculley blessing on how these recent changes impact the Docker section of CONTRIBUTING.md.

[oliviasculley]

The same commands should run in a docker install just like the host install, so those changes look good to me!

[allella]

@oliviasculley but would the Docker commands need to be run within the docker exec -it or would some commands be run directly on the local shell?

[zach2825]

Hey @allella I want to write a sh or Makefile that you can run on all deploys that will do everything necessary. I'm leaning toward a Makefile so I can break up functionality.

But for now, if you don't mind, no matter what is running this on all deploys. yarn install --frozen-lockfile; composer install; php artisan migrate;

I do plan on removing the other package.json scripts. They're cluttered and hard to reason, and you can't comment on them.

Unless you have a preference, I'm happy to do whatever is best fo you :100:

[oliviasculley]

@oliviasculley but would the Docker commands need to be run within the docker exec -it or would some commands be run directly on the local shell?

yep they would need to be, good catch

[allella]

@zach2825 alright, so yarn install --frozen-lockfile; composer install; php artisan migrate;

Do npm dev or npm prod or similar setup screens need to be run, even if only on the initial deploy?

For deployment I'm using a shell alias, but I'd be cool with a deploy.sh or Makefile. The Makefile pattern reminds me of compiling C, so I wonder if there's a more modern standard. Like is deploy.sh an actually common pattern or did I just make that up?

[allella]

@MarkMcDaniels the standard commands would be like such below, where the ; in the script is chaining and will run each command even if the one before it failed.

@zach2825 I usually use && to chain shell commands. Doesn't really matter to me, but do we want all commands to run if an earlier one failed, or are the dependent where a && makes sense?

For normal hosting yarn install --frozen-lockfile; composer install; php artisan migrate;

Docker section would be like the following

docker exec -it yarn install --frozen-lockfile; composer install; php artisan migrate

[oliviasculley]

for docker at least you'll want to use &&, if a command fails with a good configuration then most likely something isn't correct with the image and it shouldn't try to continue executing

[allella]

@MarkMcDaniels let's keep an eye on PR #153 as it has a deploy script, which we could use instead of the manual commands we've been talking about more recently.

scripts/handle-deplopy-update.sh would be run instead of the commands.

@oliviasculley if we use scripts/handle-deplopy-update.sh then would we still use docker exec -it to trigger an update within Docker, like

docker exec -it sh scripts/handle-deplopy-update.sh

?

[oliviasculley]

yep, that'd be correct

[MarkMcDaniels]

@oliviasculley && @allella So the docker section command chain should look like this?

docker exec -it sh scripts/handle-deplopy-update.sh

instead of this?

docker exec -it yarn install --frozen-lockfile; composer install; php artisan migrate

[allella]

@MarkMcDaniels I believe so, but note deploy is spelled incorrectly (deplopy) in the script name. Zach is going to fix that script name, so we can spell it correctly in the CONTRIBUTING.md too.

[oliviasculley]

Sorry I misread, it's actually docker exec -it container_name bash_command.sh, so for the examples above, it would be something like docker exec -it hackgreenville scripts/handle-deploy-update.sh

[zach2825]

I don't know if you have seen Laravel Sail, but it's impressive for helping with docker. It acts like a wrapper for PHP artisan too. So you run sail artisan tinker and sail in place for other commands. https://laravel.com/docs/10.x/sail#configuring-a-shell-alias

TThis was also announced at laracon, Iit only works for Mac thou. https://herd.laravel.com/

[allella]

@MarkMcDaniels #156 made other docs change and there are a couple minor merge conflicts now on this PR.

Though, updating to run deploy.sh is still relevant, so we could still use changes in that regard.

On the "host" part of the docs, it looks like that would be replacing the commands in the https://github.com/hackgvl/hackgreenville-com/blob/develop/CONTRIBUTING.md#installing-dependencies-and-seeding-database section with the equivalent deploy.sh

and on the "Docker" part of the docs it would be replacing stuff based on what Olivia posted above https://github.com/hackgvl/hackgreenville-com/pull/148#issuecomment-1648230504

[allella]

@MarkMcDaniels a lot of folks made other changes to the CONTRIBUTING.md and then I went and reorganized it all in #163, so I'm closing this as I added the deploy script while I was in there.

@irby the only thing we didn't cover from this issue is if we still want to tell people in the Docker section of the CONTRIBUTING.md to run the scripts/handle-deploy-update.sh, which runs a number of commands that can / should be run any time someone is setting up or syncing up or deploying.

The main question is if running docker exec -it hackgreenville scripts/handle-deploy-update.sh still makes sense like we talked about above since the Docker and Sail option my need some commands, like perhaps composer install to be run in a different order among the Docker commands.

In any case, if we need to update the docs for Docker in this regard, then let's create a new PR and reference back to this comment.