Skip to content

Conversation

@hasezoey
Copy link
Collaborator

Summary

This PR replaces many occurrences of localhost with 127.0.0.1
Exceptions:

  • everything in CHANGELOG.md
  • some exceptions in connections.md where it is talked about using localhost as a hostname

re #12741

@hasezoey hasezoey requested a review from vkarpov15 December 17, 2022 11:04
Copy link
Collaborator

@AbdelrahmanHafez AbdelrahmanHafez left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! 👍

Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

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

Thanks 👍

* `npm run docs:view`

Visit `http://localhost:8089` and you should see the docs with your local changes. Make sure you `npm run docs:clean` before committing, because automated generated files to `docs/*` should **not** be in PRs.
Visit `http://127.0.0.1:8089` and you should see the docs with your local changes. Make sure you `npm run docs:clean` before committing, because automated generated files to `docs/*` should **not** be in PRs.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This one isn't strictly necessary because Node !== browser. But shouldn't be a problem.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i saw this in the mass replace and wanted to keep it just in case a browser also decides to change that meaning and to keep the docs more consistent (or in case it is used as a argument to some node process, like a link checker)

@vkarpov15 vkarpov15 added this to the 6.8.1 milestone Dec 19, 2022
@vkarpov15 vkarpov15 merged commit 9d7bf10 into master Dec 19, 2022
@vkarpov15 vkarpov15 deleted the replaceLocalhost branch December 19, 2022 21:47
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.

5 participants