Skip to content
This repository has been archived by the owner on Jun 9, 2023. It is now read-only.

Improve Docker Mode, Manual Mode, Node Version, Database, Postegres Sections of CONTRIBUTING.md #448

Merged
merged 10 commits into from
Mar 11, 2021

Conversation

allella
Copy link
Contributor

@allella allella commented Mar 9, 2021

  • I have read Chapter's contributing guidelines.
  • My pull request has a descriptive title (not a vague title like Update README.md).
  • My pull request targets the master branch of Chapter.

This addresses most of the issues outlined in #447.

The main changes are:

  • splitting the "Contributing Code" steps from the "Running the Application" steps
  • clearly defining "Docker Mode" versus "Manual Mode" steps
  • improving the Database docs
  • making it more clear how to view the website and API docs once the app is running

I'm sure there are some typos remaining, but I wanted to get this in front of others for review to make sure the overall direction is agreeable.

@gitpod-io
Copy link

gitpod-io bot commented Mar 9, 2021

@allella
Copy link
Contributor Author

allella commented Mar 9, 2021

@paizle @dtwilliams10 I could use fresh eyes on this if you have time to review it.

Github won't let me tag reviewers until they are involved in the conversation.

@allella
Copy link
Contributor Author

allella commented Mar 9, 2021

I am curious if "Step 1 - Install Node and Run npx" of the Running the Application section is specific to the Manual Mode or if Docker Mode also needs Node installed locally. I'd assume the Docker container would install node, but I don't know Docker or Node and haven't tested Manual Mode.

@paizle
Copy link
Contributor

paizle commented Mar 9, 2021

@allella I might be in the same boat as you with regards to Docker and node. Right now I haven't gotten the Docker setup working so I'm using the manual setup. I was working a bit with Tom Noland (hope I'm spelling that right) on the Docker setup and got a bit further with his help but I think it was still a work in progress and wasn't fully working. I'm not sure if the problems might be with my git bash on windows platform or not.

@paizle
Copy link
Contributor

paizle commented Mar 9, 2021

I am curious if "Step 1 - Install Node and Run npx" of the Running the Application section is specific to the Manual Mode or if Docker Mode also needs Node installed locally. I'd assume the Docker container would install node, but I don't know Docker or Node and haven't tested Manual Mode.

I'm just testing that now with a fresh install. I think if you are using docker then you might only need to cd into the client directory and npm install there. I tried docker-compose up in the root without using npx recursive-install first and got an error here relating to next not being installed:
client_1 | $ next
client_1 | /bin/sh: next: not found
client_1 | error Command failed with exit code 127.

@allella
Copy link
Contributor Author

allella commented Mar 9, 2021

I just ran a test on a fresh install and npx recursive-install a necessary prerequisite for Docker Mode.

It's not a big deal, I just figured everything would run inside Docker and it may only be needed for Manual Mode. I suspect there's a way to move that npx / next stuff into the Docker Compose or Dockerfile, but that's beyond me at this point and it's a trivia step to run, so not going to worry about it.

client_1  | $ next
client_1  | /bin/sh: next: not found
client_1  | error Command failed with exit code 127.
client_1  | info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
chapter-npx-test_client_1 exited with code 127

@paizle
Copy link
Contributor

paizle commented Mar 9, 2021

It could be noted in the docs that using the local db step requires yarn db:reset as well.

@allella
Copy link
Contributor Author

allella commented Mar 9, 2021

@paizle "Step 3 - Prepare the Database for Development" under Running the Application mentioned the need to run tasks on the database and links to the relevant section lower on the page, which includes the reset, migrate, and see commands. Is that sufficient to address the suggestion of db:reset?

@allella allella merged commit cad59d3 into freeCodeCamp:master Mar 11, 2021
@allella allella deleted the docs/contributing-md-docs-tweaks branch March 11, 2021 22:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants