Skip to content

Adding hashes to links#1729

Closed
mrienstra wants to merge 2 commits into
withastro:mainfrom
mrienstra:patch-1
Closed

Adding hashes to links#1729
mrienstra wants to merge 2 commits into
withastro:mainfrom
mrienstra:patch-1

Conversation

@mrienstra
Copy link
Copy Markdown
Contributor

What kind of changes does this PR include?

  • Minor content fixes (broken links, typos, etc.) <-- mostly
  • New or updated content <-- a smidgen

This PR began with adding hashes to links.

Along the way, I found & fixed / improved:

  • Added extendDefaultPlugins: true to 2 examples, to reduce surprising side effects that may otherwise occur when blindly copying code from examples.
  • Indentation.
  • npm i --> npm install (for consistency).
  • Changed and added a few links.
  • Removed unnecessary import Tabs from 2 pages
  • Switched 1 page from Tabs to PackageManagerTabs
  • Added PackageManagerTabs to 1 page.
  • Added a link from reference/adapter-reference to the SSR guide, as there was otherwise no convenient path from reference/adapter-reference to other adapter docs.
  • Added some new headers to guides/server-side-rendering, in part so I could link directly to #adding-an-adapter, but also because I think it will scan better.

During the same editing session, I edited 4 auto-generated files:
main...mrienstra:docs:placeholder-1

I'll have to open another PR for those changes, the sources in question being:

and fixing random stuff
@netlify
Copy link
Copy Markdown

netlify Bot commented Oct 5, 2022

Deploy Preview for astro-docs-2 ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 8eb5385
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/633e1baab1b39d00094a2ea3
😎 Deploy Preview https://deploy-preview-1729--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 settings.

@mrienstra
Copy link
Copy Markdown
Contributor Author

Whoops, removed Tabs import that was being used, fixing that now...

@github-actions github-actions Bot added the i18n Anything to do with internationalization & translation efforts - ask @YanThomas for help! label Oct 6, 2022
@mrienstra
Copy link
Copy Markdown
Contributor Author

mrienstra commented Oct 6, 2022

Find:

<Tabs client:visible sharedStore="js-ts">\n<Fragment slot="tab\.js">JavaScript</Fragment>\n<Fragment slot="tab\.ts">TypeScript</Fragment>\n<Fragment slot="panel\.js">\n([\w\W]+?)\n</Fragment>\n<Fragment slot="panel\.ts">\n([\w\W]+?)\n</Fragment>\n</Tabs>

Replace:

<JavascriptFlavorTabs>
  <Fragment slot="js">
  $1
  </Fragment>
  <Fragment slot="ts">
  $2
  </Fragment>
</JavascriptFlavorTabs>

Find:

<Tabs client:visible>\n  <Fragment slot="tab.1.npm">npm</Fragment>\n  <Fragment slot="tab.2.yarn">yarn</Fragment>\n  <Fragment slot="tab.3.pnpm">pnpm</Fragment>\n  <Fragment slot="panel.1.npm">\n  ([\w\W]+?)\n  </Fragment>\n  <Fragment slot="panel.2.yarn">\n  ([\w\W]+?)\n  </Fragment>\n  <Fragment slot="panel.3.pnpm">\n  ([\w\W]+?)\n  </Fragment>\n</Tabs>

Replace:

<PackageManagerTabs>
  <Fragment slot="npm">
  $1
  </Fragment>
  <Fragment slot="pnpm">
  $2
  </Fragment>
  <Fragment slot="yarn">
  $3
  </Fragment>
</PackageManagerTabs>

PS: Happy to back out /es/, /pt-br/ & /zh-cn/ changes, my thinking was that it would be permissible as I didn't change any translated copy.

@sarah11918
Copy link
Copy Markdown
Member

Hi @mrienstra, lots of stuff here! I appreciate how much effort you're putting into all kinds of docs improvements!

This is a LOT for one PR, and not something we'd normally encourage putting all together like this. Partly because some people might be appropriate to look at some, but not worry about all of it. And protip: if the title of the PR doesn't describe what's in here, that's a clue that it's probably more than we want to address at once! ;)

At the very least, could I ask you to separate out into separate PRs:

  1. adding a new <Tab /> component, suggesting the use of a tab component where one wasn't and/or removing or changing to use a different kind of tab component
  2. adding content to code samples that you believe is an improvement, including the added properties, changing i to install etc.
    and then
    ... you have several content suggested improvements, not all of which relate to each other. I'd ask you to separate out these into meaningful batches of changes in different PRs, using the title of the PR as a guide to let us know what the intention is behind them.

A PR like this makes a lot of simple quick fixes actually harder to get through, as any one of the maintainers could quickly handle one of these changes, and the title can be a reasonable expectation of who is up for reviewing knowing what the PR contains in terms of content and scope. When that alternative exists, PRs like this just don't make a lot of sense for our community workflow.

Remember, PRs are freeeee! 🥳 So please do in future try to keep changes contained to the PR title. You can decide whether you'd like to keep this one open for one of the PRs, and remove a bunch of the changes, or whether you'd like to close this one and start fresh with individual PRs for everything.

@mrienstra mrienstra closed this Oct 10, 2022
@mrienstra
Copy link
Copy Markdown
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

i18n Anything to do with internationalization & translation efforts - ask @YanThomas for help!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants