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: add v5.0.0 migration guide (fixes #10142) #10147

Merged
merged 6 commits into from Mar 30, 2018

Conversation

Projects
None yet
4 participants
@not-an-aardvark
Member

not-an-aardvark commented Mar 30, 2018

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

[x] Documentation update

What changes did you make? (Give an overview)

This adds a migration guide for v5.0.0.

A rendered version can be found here.

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

Are there any sections that are unclear?

@mysticatea

Thank you for writing.
I can't suggest about English, but just I found some points:

  1. About #7358 looks lacking.
  2. Some anchor tags are illegal.
  3. "Empty files are now linted" is related to also plugin/custom rule developers.

Thank you.

---
## <a name="drop-node-4"/> Node.js 4 is no longer supported

This comment has been minimized.

@mysticatea

mysticatea Mar 30, 2018

Member

This anchor tag is illegal since HTML (the preview shows with <!DOCTYPE html>) disallows self-closing signs in non-void elements. In fact, the anchor tag looks to span from this header to the next header in the preview, as it's cloned three times.

I think this needs that end tag:

## <a name="drop-node-4">Node.js 4 is no longer supported</a>
ESLint v5 treats whitespace-only files the same way as all other files: it parses them and runs enabled rules on them as appropriate. This could lead to additional linting problems if you use a custom rule that reports errors on empty files.
**To address:** If you have an empty file in your project and you don't want it to be linted, consider adding it to an [`.eslintignore` file](/docs/user-guide/configuring#ignoring-files-and-directories).

This comment has been minimized.

@mysticatea

mysticatea Mar 30, 2018

Member

I think this change is related to also plugin/custom rule developers. They have to update rules which have problems on empty files.

@platinumazure

Outside of mysticatea's comments, LGTM, thanks!

not-an-aardvark added some commits Mar 30, 2018

@btmills

Just two minor tweaks - one an optional suggestion and the other a typo. LGTM otherwise!

**To address:** Make sure you upgrade to at least Node.js 6 when using ESLint v5. If you are unable to upgrade, we recommend continuing to use ESLint v4.x until you are able to upgrade Node.js.
*Note: Node.js 4 can still be used with the latest alpha release of ESLint v5, but we plan to officially drop support for it before the first stable release.*

This comment has been minimized.

@btmills

btmills Mar 30, 2018

Member

Would it be worth changing this to "The latest alpha release of ESLint v5 may still function on Node.js 4, but..." to make it clear that if it does happen to appear functional on Node v4 still, that's not intentional and not something that can be relied upon?

Previously, ESLint would set the `parent` property on each AST node immediately before running rule listeners for that node. This caused some confusion for rule authors, because the `parent` property would not initially be present on any nodes, and it was sometimes necessary to complicate the structure of a rule to ensure that the `parent` property of a given node would be available when needed.
In ESLint v5, the `parent` property is set on all AST nodes before any rules have access to the AST. This makes it easier to write some rules, because the `parent` property is always available rather than being mutated behind the scenes. However, as a side-effect of having `parent` properties, the AST object has a circular structure the first time a rule sees it (previously, it only have a circular structure after the first rule listeners were called). As a result, a custom rule that enumerates all properties of an node in order to traverse the AST might now loop forever or run out of memory if it does not check for cycles properly.

This comment has been minimized.

@btmills

btmills Mar 30, 2018

Member

"previously, it only havehad"

@btmills

Just two minor tweaks - one an optional suggestion and the other a typo. LGTM otherwise!

@btmills

Just two minor tweaks - one an optional suggestion and the other a typo. LGTM otherwise!

not-an-aardvark added some commits Mar 30, 2018

@not-an-aardvark not-an-aardvark merged commit 3351129 into master Mar 30, 2018

5 checks passed

commit-message PR title follows commit message guidelines
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details
release-monitor No patch release is pending
Details

@not-an-aardvark not-an-aardvark deleted the v5-migration-guide branch Mar 30, 2018

@eslint eslint bot locked and limited conversation to collaborators Sep 27, 2018

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