-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Update readme for Netlify and NextJS #12041
Conversation
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Overall looks good! Just a comment about the Netlify preview screenshot image
README.md
Outdated
``` | ||
|
||
- Open this directory in your favorite text editor / IDE, and see your changes live by visiting `localhost:8000` from your browser | ||
- Open this directory in your favorite text editor / IDE, and see your changes live by visiting `localhost:3000` from your browser | ||
- Pro Tip: | ||
- Explore scripts within `package.json` for more build options | ||
- Get **faster** local builds by building only one language. E.g. in your `.env` file, set `BUILD_LOCALES=en` to build the content only in English |
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.
Not a blocker for this PR, but this tip is less helpful now that we use Next.js... it's handy if you're building the site, but doesn't matter for yarn dev
since pages are built on-demand.
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.
Agree. I found it informative though. Perhaps we could clarify that it is faster for production builds (yarn build
).
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.
Changed the wording here to be production
README.md
Outdated
- Gatsby Cloud (our hosting service for build previews) deploys all PRs to a publicly accessible preview URL, e.g.: | ||
![Gatsby Cloud deploy preview](./GC-preview-deploy.png) | ||
- _Confirm your GC preview deploy looks & functions as expected_ | ||
- Netlify (our hosting service for build previews) deploys all PRs to a publicly accessible preview URL, e.g.: |
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.
...accessible preview URL, e.g.:
@corwintines Did we mean to remove the image here? If so, we should remove the , e.g.:
from the end of this line
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.
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.
Added this image, thanks @pettinarip!
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.
Nicely done @corwintines left a couple of comments.
README.md
Outdated
- Gatsby Cloud (our hosting service for build previews) deploys all PRs to a publicly accessible preview URL, e.g.: | ||
![Gatsby Cloud deploy preview](./GC-preview-deploy.png) | ||
- _Confirm your GC preview deploy looks & functions as expected_ | ||
- Netlify (our hosting service for build previews) deploys all PRs to a publicly accessible preview URL, e.g.: |
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.
Co-authored-by: Pablo Pettinari <[email protected]>
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.
lgtm! Thanks @corwintines!
Description
Fixes: #12068