Skip to content
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

README improvements and fixes #166

Merged
merged 2 commits into from
Nov 8, 2023
Merged

README improvements and fixes #166

merged 2 commits into from
Nov 8, 2023

Conversation

mabuyo
Copy link
Contributor

@mabuyo mabuyo commented Nov 1, 2023

As I was going through the instructions to get this running locally, I ran into a couple issues and realized the README was missing some steps or could use more clarity.

Copy link

codesandbox bot commented Nov 1, 2023

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

Copy link

netlify bot commented Nov 1, 2023

Deploy Preview for comforting-syrniki-15960f canceled.

Name Link
🔨 Latest commit b0640e3
🔍 Latest deploy log https://app.netlify.com/sites/comforting-syrniki-15960f/deploys/654290a69bfda900082aed6f

APOLLO_KEY={YOUR_API_KEY} npm run graphos-demo
```
```sh
AUTH={YOUR_API_KEY} npm run graphos-demo
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scipts/demo.ts was looking for process.env.AUTH in line 16. So I changed this accordingly. Should we change the script instead?


**\*Note**: We're currently working on subscriptions support with `rover dev` so the app is pointing at the deployed production url when running locally. You can change the URL the client application is pointing at by editing the `.env.development` file with `VITE_SERVER_HOST` set to the desired URL (most likely locally in the next steps)
Local subscriptions using `rover dev` is currently a work-in-progress, so features that use subscriptions are not functional (such as listening to playback state). You can change the URL the client application is pointing at by editing the `.env.development` file with `VITE_SERVER_HOST` and setting it to the production URL (uncomment the second line).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The instructions are actually downloading the router and running it. Do local subscriptions work that way? Should we remove this rover dev note?

CALLBACK_URL=http://router:4000
```
```
APOLLO_GRAPH_REF={YOUR_DEMO_GRAPH_ID}@main
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing main as the variant when I ran the script, not prod.

Comment on lines +158 to +169
```
npm run build
```

5. In a new terminal, start the router:
```
npm run start:spotify && npm run start:playback
```

```
APOLLO_KEY={YOUR_DEMO_GRAPH_ID}@prod APOLLO_GRAPH_REF={YOUR_GRAPH_API_KEY} CALLBACK_URL=http://127.0.0.1:4000 npm run start:router
```
- Spotify Subgraph - http://localhost:4001
- Playback Subgraph - http://localhost:4002

> **Note:** If you make any edits to the subgraph files, you'll need to re-build and restart the subgraphs manually. If you're looking to do local development, you may want to navigate to each subgraph folder and run `npm run dev` instead, which will watch for changes and restart the subgraph automatically.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to build the subgraphs, otherwise the start commands wouldn't work.


5. Visit the website at http://localhost:3000
By default, the client app is pointing to the _locally-running_ backend URL. Follow the section below for "I want to run the backend locally" (you can choose to use Docker, or not).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original docs said it was pointing to the deployed prod URL, but that was actually untrue. Should we change it to point to the deployed prod? I changed the README to reflect the current state.

@@ -1,5 +1,5 @@
VITE_SERVER_HOST=http://localhost:4000
# https://spotify-showcase-production-d157.up.railway.app/
# VITE_SERVER_HOST=https://spotify-showcase-production-d157.up.railway.app/ # uncomment this to use the production URL
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making it clearer what this URL can be used for

@mabuyo mabuyo marked this pull request as ready for review November 1, 2023 17:45
@michael-watson michael-watson merged commit 3bac639 into main Nov 8, 2023
6 checks passed
@michael-watson michael-watson deleted the mm/readme-impr branch November 8, 2023 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants