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

docs: IA Update page URL move #16665

Merged
merged 41 commits into from Jan 15, 2023
Merged

docs: IA Update page URL move #16665

merged 41 commits into from Jan 15, 2023

Conversation

bpmutter
Copy link
Contributor

@bpmutter bpmutter commented Dec 15, 2022

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[x] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

move page URLs to locations in the New IA and internal links to those pages.

Is there anything you'd like reviewers to focus on?

❗❗❗ Please Read Before Reviewing! ❗❗❗

While I think it'll be useful to do a manual check, given the large number of files and links, I found using automated link-checking tools helpful as well. I would recommend that the reviewers check using a tool as well. The tools are:

  • Semrush - This was the most useful tool with the best data, but it's paid (you can use the 1 week free trial, which is what I did)
  • Broken Link Checker - free tool

Once the changes have been approved the changes to the following files must be reverted:

  • docs/src/static/robots.njk
  • docs/src/_includes/layouts/base.html

These changes were made to let the link checker tools parse the staging site.

Also, this PR should be reviewed, merged, and deployed together with eslint/eslint.org#388. This PR is for the redirects associated with the IA changes.

Also, I'm not sure why but the PR says I have conflicting files (see below). Not sure why as I've rebased the changes on top of the current main branch. would appreciate any ideas on how to fix. (edit: resolved)

@eslint-github-bot eslint-github-bot bot added the triage An ESLint team member will look at this issue soon label Dec 15, 2022
@eslint-github-bot
Copy link

Hi @bpmutter!, thanks for the Pull Request

The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

  • The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
  • There should be a space following the initial tag and colon, for example 'feat: Message'.

To Fix: You can fix this problem by running git commit --amend, editing your commit message, and then running git push -f to update this pull request.

Read more about contributing to ESLint here

@netlify
Copy link

netlify bot commented Dec 15, 2022

Deploy Preview for docs-eslint ready!

Name Link
🔨 Latest commit 3e50f22
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/63b9d3505a203d000905f7ff
😎 Deploy Preview https://deploy-preview-16665--docs-eslint.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.

@@ -4,4 +4,3 @@ permalink: robots.txt
eleventyExcludeFromCollections: true
---
User-agent: *
Disallow: /
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note
Remove this change before merge. needed for URL checkers to parse.

Copy link
Member

Choose a reason for hiding this comment

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

This might actually be a mistake. I think we were supposed to remove Disallow: / once we went live.

Copy link
Contributor Author

@bpmutter bpmutter Dec 28, 2022

Choose a reason for hiding this comment

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

yeah i was confused by that.

it's like this on the prod site as well. in https://eslint.org/docs/latest/robots.txt, the contents are:

User-agent: *
Disallow: /

upon discovering this, i was confused how google would index the eslint docs at all. but upon some investigation, found this page, which explains that google indexes if there are backlinks from other sites to this one, even if robots.txt disallows indexing.

wonder if keeping this change would improve SEO for the docs.

Copy link
Member

Choose a reason for hiding this comment

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

This prevents crawling of https://latest--docs-eslint.netlify.app, https://docs-eslint.netlify.app, deploy previews and other URLs where robots.txt will be in the root, while having no effect on https://eslint.org/ since it isn't in the root there.

Copy link
Member

Choose a reason for hiding this comment

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

Ah right. Thanks.

@bpmutter bpmutter marked this pull request as ready for review December 21, 2022 03:08
@bpmutter bpmutter requested a review from a team as a code owner December 21, 2022 03:08
@bpmutter bpmutter mentioned this pull request Dec 21, 2022
45 tasks
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

A few notes from the first pass:

  • nodejs-api.md should be in integrate/
  • code-path-analysis.md should be in extend/
  • There's an empty file extend/unit-tests

@mdjermanovic
Copy link
Member

Also, I'm not sure why but the PR says I have conflicting files (see below). Not sure why as I've rebased the changes on top of the current main branch. would appreciate any ideas on how to fix.

Try rebasing again. This branch is currently 18 commits behind main and the 4 conflicting files have been changed in those 18 commits: bpmutter/eslint@ia_updates...eslint:eslint:main

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
lib/rule-tester/rule-tester.js Outdated Show resolved Hide resolved
lib/rule-tester/rule-tester.js Outdated Show resolved Hide resolved
lib/rule-tester/rule-tester.js Outdated Show resolved Hide resolved
@bpmutter
Copy link
Contributor Author

bpmutter commented Jan 6, 2023

@mdjermanovic comments resolved in 869a5c6

@mdjermanovic
Copy link
Member

Looks good!

I think you can now revert changes in base.html and robots.njk to make this ready for merge.

@mdjermanovic
Copy link
Member

As for merging this PR, how shall we proceed?

  • If we merge this now (as soon as docs: IA Update page URL move #16665 (comment) is addressed), "Edit this page" links on the website and some links in README.md and CONTRIBUTING.md on GitHub will be broken until the next release.
  • If we wait and merge this right before the release, given the number of moved files, there will likely be merge conflicts so it might be good to freeze the docs until then to avoid fixing the conflicts here.

@bpmutter
Copy link
Contributor Author

bpmutter commented Jan 7, 2023

@mdjermanovic applied those 2 reversions.

and regarding:

As for merging this PR, how shall we proceed?

  • If we merge this now (as soon as docs: IA Update page URL move #16665 (comment) is addressed), "Edit this page" links on the website and some links in README.md and CONTRIBUTING.md on GitHub will be broken until the next release.
  • If we wait and merge this right before the release, given the number of moved files, there will likely be merge conflicts so it might be good to freeze the docs until then to avoid fixing the conflicts here.

i think we should choose the 2nd option. i don't see an issue with that. and i can still start on some further docs changes, just based on top of the branch in this PR.

and if there are any additional community docs contributions between now and the merge of this, we can tell them to put the changes on top of this PR's branch. i'll also be happy to help with merging these community changes, should there be any.

@nzakas
Copy link
Member

nzakas commented Jan 11, 2023

I think we can do it right before the release 👍

Copy link
Member

@mdjermanovic mdjermanovic 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!

This should be merged right before the release.

Then, right after the release, eslint/eslint.org#388 should be merged.

@btmills btmills merged commit 17b65ad into eslint:main Jan 15, 2023
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Jan 17, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.31.0` -> `8.32.0`](https://renovatebot.com/diffs/npm/eslint/8.31.0/8.32.0) |

---

### Release Notes

<details>
<summary>eslint/eslint</summary>

### [`v8.32.0`](https://github.com/eslint/eslint/releases/tag/v8.32.0)

[Compare Source](eslint/eslint@v8.31.0...v8.32.0)

#### Features

-   [`fc20f24`](eslint/eslint@fc20f24) feat: add suggestions for redundant wrapping in prefer-regex-literals ([#&#8203;16658](eslint/eslint#16658)) (YeonJuan)

#### Bug Fixes

-   [`b4f8329`](eslint/eslint@b4f8329) fix: ignore directives for no-fallthrough ([#&#8203;16757](eslint/eslint#16757)) (gfyoung)

#### Documentation

-   [`17b65ad`](eslint/eslint@17b65ad) docs: IA Update page URL move ([#&#8203;16665](eslint/eslint#16665)) (Ben Perlmutter)
-   [`5981296`](eslint/eslint@5981296) docs: fix theme switcher button ([#&#8203;16752](eslint/eslint#16752)) (Sam Chen)
-   [`6669413`](eslint/eslint@6669413) docs: deploy prerelease docs under the `/docs/next/` path ([#&#8203;16541](eslint/eslint#16541)) (Nitin Kumar)
-   [`78ecfe0`](eslint/eslint@78ecfe0) docs: use inline code for rule options name ([#&#8203;16768](eslint/eslint#16768)) (Percy Ma)
-   [`fc2ea59`](eslint/eslint@fc2ea59) docs: Update README (GitHub Actions Bot)
-   [`762a872`](eslint/eslint@762a872) docs: Update README (GitHub Actions Bot)

#### Chores

-   [`2952d6e`](eslint/eslint@2952d6e) chore: sync templates/\*.md files with issue templates ([#&#8203;16758](eslint/eslint#16758)) (gfyoung)
-   [`3e34418`](eslint/eslint@3e34418) chore: Add new issues to triage project ([#&#8203;16740](eslint/eslint#16740)) (Nicholas C. Zakas)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xMDIuNCIsInVwZGF0ZWRJblZlciI6IjM0LjEwMi43In0=-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1734
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
kecrily added a commit to eslint/zh-hans.docs.eslint.org that referenced this pull request Feb 20, 2023
kecrily added a commit to eslint/zh-hans.docs.eslint.org that referenced this pull request Feb 28, 2023
kecrily added a commit to eslint/zh-hans.docs.eslint.org that referenced this pull request Feb 28, 2023
kecrily added a commit to eslint/zh-hans.docs.eslint.org that referenced this pull request Feb 28, 2023
kecrily added a commit to eslint/zh-hans.docs.eslint.org that referenced this pull request Feb 28, 2023
* docs: add options to check destructuring in no-underscore-dangle

eslint/eslint#16006

* docs: check assignment patterns in no-underscore-dangle

eslint/eslint#16693

* docs: use inline code for rule options name

eslint/eslint#16768

* docs: ignore directives for no-fallthrough

eslint/eslint#16757

* docs: IA Update page URL move

eslint/eslint#16665

* update

* feat: sync v8.31.0 (#94)

* docs: User Guide Getting Started expansion

* docs: add options to check destructuring in no-underscore-dangle

eslint/eslint#16006

* docs: adjust some words

* docs: Add function call example for no-undefined

eslint/eslint#16712

* docs: check assignment patterns in no-underscore-dangle

eslint/eslint#16693

* update formatters

* Apply suggestions from code review

Co-authored-by: Strek <ssharishkumar@gmail.com>

---------

Co-authored-by: Strek <ssharishkumar@gmail.com>

* docs: add options to check destructuring in no-underscore-dangle

eslint/eslint#16006

* docs: IA Update page URL move

eslint/eslint#16665

---------

Co-authored-by: Strek <ssharishkumar@gmail.com>
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Jul 15, 2023
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Jul 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants