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

[i18nIgnore] feat: change default port #3977

Merged
merged 9 commits into from
Aug 30, 2023

Conversation

ematipico
Copy link
Member

@ematipico ematipico commented Jul 31, 2023

What kind of changes does this PR include?

  • New or updated content

Description

This PR changes the default port to 4321.

Code PR: withastro/astro#7874

I only did a search and replace of :3000 throughout the website. Let me know if the change is too disruptive and if I should change only a subset of pages.

@netlify
Copy link

netlify bot commented Jul 31, 2023

Deploy Preview for astro-docs-2 ready!

Name Link
🔨 Latest commit ca6e039
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/64ef4392ff8b260008c0ca12
😎 Deploy Preview https://deploy-preview-3977--astro-docs-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions github-actions bot added the i18n Anything to do with internationalization & translation efforts - ask @YanThomas for help! label Jul 31, 2023
@ematipico ematipico added the v3.0 label Jul 31, 2023
Copy link
Member

@ElianCodes ElianCodes left a comment

Choose a reason for hiding this comment

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

Just left a couple of changes, mostly generalising the use of http://localhost:4321 instead of localhost:4321 or https://localhost:4321

Sorry in advance 🤣

src/content/docs/en/tutorial/1-setup/2.mdx Outdated Show resolved Hide resolved
src/content/docs/en/tutorial/6-islands/2.mdx Outdated Show resolved Hide resolved
src/content/docs/en/tutorial/5-astro-api/4.mdx Outdated Show resolved Hide resolved
src/content/docs/en/tutorial/5-astro-api/3.mdx Outdated Show resolved Hide resolved
src/content/docs/en/tutorial/5-astro-api/3.mdx Outdated Show resolved Hide resolved
src/content/docs/ru/tutorial/5-astro-api/3.mdx Outdated Show resolved Hide resolved
src/content/docs/ru/tutorial/5-astro-api/3.mdx Outdated Show resolved Hide resolved
src/content/docs/ru/tutorial/5-astro-api/4.mdx Outdated Show resolved Hide resolved
src/content/docs/ru/tutorial/6-islands/2.mdx Outdated Show resolved Hide resolved
src/content/docs/ru/tutorial/6-islands/2.mdx Outdated Show resolved Hide resolved
@ematipico
Copy link
Member Author

@ElianCodes I leave the task to merge the suggestions to you :)

@ElianCodes
Copy link
Member

@ematipico allright! Can you confirm that the port will default to 4322 instead of 4321 when that port is occupied?

@ematipico
Copy link
Member Author

@ematipico allright! Can you confirm that the port will default to 4322 instead of 4321 when that port is occupied?

Yes :)

Same behaviour as before. Astro starts looking for the first port available, incrementally: 4321, 4322, 4323, etc.

@ElianCodes
Copy link
Member

Hey @ematipico! I'm in the process of generalising the protocol. Gonna go through all of them one last time.

When I'm done, I'll keep you posted!

Copy link
Member

@ElianCodes ElianCodes left a comment

Choose a reason for hiding this comment

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

Found a couple of leftovers 😱

src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
src/content/docs/en/tutorial/2-pages/2.mdx Outdated Show resolved Hide resolved
src/content/docs/en/tutorial/2-pages/2.mdx Outdated Show resolved Hide resolved
src/content/docs/en/tutorial/4-layouts/2.mdx Outdated Show resolved Hide resolved
src/content/docs/es/tutorial/2-pages/2.mdx Outdated Show resolved Hide resolved
src/content/docs/es/tutorial/2-pages/2.mdx Outdated Show resolved Hide resolved
src/content/docs/es/tutorial/5-astro-api/2.mdx Outdated Show resolved Hide resolved
src/content/docs/ja/tutorial/1-setup/2.mdx Outdated Show resolved Hide resolved
src/content/docs/ru/tutorial/1-setup/2.mdx Outdated Show resolved Hide resolved
Copy link
Member

@ElianCodes ElianCodes left a comment

Choose a reason for hiding this comment

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

I think I now should have all of them! Good to go!

@sarah11918 sarah11918 added add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. and removed i18n Anything to do with internationalization & translation efforts - ask @YanThomas for help! labels Aug 5, 2023
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

WHEW! 😅

Assuming that there is otherwise no language changed here on any of the pages, just 3000 -> 4321, then I'm approving this one as ready for v3!

(If there is anything other than that I should take a peek at, please point it out to me. Otherwise, this one just waits!)

@ematipico
Copy link
Member Author

WHEW! 😅

Assuming that there is otherwise no language changed here on any of the pages, just 3000 -> 4321, then I'm approving this one as ready for v3!

(If there is anything other than that I should take a peek at, please point it out to me. Otherwise, this one just waits!)

@sarah11918

@ElianCodes changed all the instances of localshost to http://localhost. It wasn't part of the original PR.

Copy link
Member

@yanthomasdev yanthomasdev left a comment

Choose a reason for hiding this comment

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

🚨 IMPORTANT 🚨

Till this PR gets merged we will probably have more pages with localhost:3000, especially in translations. From what I can see, this PR only changes the mentions of the port, no new docs added, so the final merge message should contain [i18nIgnore] to avoid our tracker from marking these pages as 100% updated, and also rerun the search and replace to account for newer translations added.

@sarah11918
Copy link
Member

@Yan-Thomas I'm changing most of these PRs to PR the upgrade guide v3 branch instead of main, so does that solve the above problem? Let me know how we should handle this one! Maybe an intentional rescan in that PR branch shortly before merging, and then we might catch all of them?

@yanthomasdev
Copy link
Member

@Yan-Thomas I'm changing most of these PRs to PR the upgrade guide v3 branch instead of main, so does that solve the above problem?

No! Keep this PR to main, after the 3.0 PR is merged then update branch and search and replace localhost:3000 here, and then you're allowed to merge here! With [i18nIgnore]

@github-actions github-actions bot added the i18n Anything to do with internationalization & translation efforts - ask @YanThomas for help! label Aug 19, 2023
@sarah11918 sarah11918 marked this pull request as draft August 25, 2023 18:59
@yanthomasdev yanthomasdev marked this pull request as ready for review August 30, 2023 13:26
@yanthomasdev yanthomasdev changed the title feat: change default port [i18nIgnore] feat: change default port Aug 30, 2023
@yanthomasdev yanthomasdev merged commit e97f88e into main Aug 30, 2023
12 checks passed
@yanthomasdev yanthomasdev deleted the feat/change-default-port-plt-739 branch August 30, 2023 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. i18n Anything to do with internationalization & translation efforts - ask @YanThomas for help! v3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants