Add URL to rule documentation to the metadata #998

Arcanemagus · 2018-01-08T18:51:59Z

ESLint v4.15.0 added an official location for rules to store a URL to their documentation in the rule metadata in eslint/eslint#9788. This adds the URL to all the existing rules so anything consuming them can
know where their documentation is without having to resort to external packages to guess.

This also fixes the docs property of no-self-import, which was previously doc.

I'm not sure I like how the URL's are being defined here to fit within the 99 character line length limit, if you would like me to move the ruleDocsUrl to a single file that they all import, or alternatively do something like eslint-community/eslint-plugin-eslint-plugin@eb207d0, just let me know!

Note: imports-first has a slightly different format since the documentation for this deprecated rule was removed. If you want that to instead point to first's documentation that can be changed.

ESLint v4.15.0 added an official location for rules to store a URL to their documentation in the rule metadata in eslint/eslint#9788. This adds the URL to all the existing rules so anything consuming them can know where their documentation is without having to resort to external packages to guess.

Arcanemagus · 2018-01-08T19:55:49Z

Build failures look to be "normal" (AppVeyor), or related to network timeouts on Travis CI (#2428.5, #2428.6, #2428.13).

ljharb

definitely don't worry about the travis timeouts, they're having trouble this week.

Thanks!

ljharb · 2018-01-08T20:18:20Z

src/rules/default.js

@@ -1,8 +1,12 @@
 import Exports from '../ExportMap'

+const ruleDocsUrl = 'https://github.com/benmosher/eslint-plugin-import/tree/master/docs/rules'


Can we import this from somewhere rather than repeating it everywhere?

Maybe something like docsUrl('rule-name')?

Yep, can do that for everything other than imports-first, since it needs a specific blob.

I didn't really like how this was fitting in the style here which is why I explicitly called it out as something to look at 😉.

Move the common string of the URL to a function that is called. Note that `imports-first` still has a fully defined URL in the rule file as the documentation for that rule requires a specific blob id since it has been deleted on `master`.

Arcanemagus · 2018-01-08T20:47:59Z

Updated, let me know if there are any other changes 😉.

ljharb · 2018-01-08T21:19:55Z

src/rules/imports-first.js

+const newMeta = Object.assign({}, first.meta, {
+  deprecated: true,
+  docs: {
+    url: `${ruleDocsUrl}/7b25c1cb95ee18acc1531002fd343e1e6031f9ed/docs/rules/imports-first.md`,


i'm confused, why does this one have a sha but the others don't? could it be an optional second argument passed to docsUrl?

The rule was deprecated / renamed in 8e35cd9, however the documentation was directly renamed, leaving nothing in it's place. The straight mapping used elsewhere can't be used here because of this.

An alternative here would be to simply point this rule at first's documentation, as outlined in the PR description.

Can we modify docsUrl to take an optional second argument that defaults to master but can be overridden to this sha?

The URL's are slightly different, but sure I can fix it to that.

Gave the function an optional second parameter, added tests for it, and used it for imports-first.

coveralls · 2018-01-08T22:06:41Z

Coverage decreased (-1.1%) to 95.061% when pulling a56c1c0 on Arcanemagus:rules-docs-url into 0d44914 on benmosher:master.

Add an optional second argument to `docsUrl` specifying a commit hash that, when specified, changes the returned URL to be specific to that hash instead of directing to `master`. Adds tests for `docsUrl` to ensure that it is operating as expected.

coveralls · 2018-01-08T22:54:21Z

Coverage increased (+0.02%) to 96.23% when pulling f12b6d0 on Arcanemagus:rules-docs-url into 0d44914 on benmosher:master.

j-f1 · 2018-01-08T23:08:48Z

src/docsUrl.js

+const repoUrl = 'https://github.com/benmosher/eslint-plugin-import'
+
+export default function docsUrl(ruleName, commitHash) {
+  let baseUrl = `${repoUrl}/tree/master/docs/rules`


This can be blob/master, too. tree is used for directories, while blob is used for files.

Wow, it looks like this is a pretty common mistake since GitHub automatically redirects the wrong link, but you are absolutely correct.

The `/tree/` links are meant for directories, `/blob/` leads to an exact file. This allowed a simplification of the `docsUrl` function since the default branch can be treated as a "commit hash".

coveralls · 2018-01-09T00:25:08Z

Coverage increased (+0.01%) to 96.225% when pulling c05cddb on Arcanemagus:rules-docs-url into 0d44914 on benmosher:master.

coveralls · 2018-01-09T01:09:42Z

Coverage increased (+0.02%) to 96.23% when pulling f12b6d0 on Arcanemagus:rules-docs-url into 0d44914 on benmosher:master.

coveralls · 2018-01-09T01:09:42Z

Coverage increased (+0.02%) to 96.23% when pulling f12b6d0 on Arcanemagus:rules-docs-url into 0d44914 on benmosher:master.

coveralls · 2018-01-09T01:09:43Z

Coverage increased (+0.02%) to 96.23% when pulling f12b6d0 on Arcanemagus:rules-docs-url into 0d44914 on benmosher:master.

coveralls · 2018-01-09T02:52:19Z

Coverage increased (+0.01%) to 96.225% when pulling c05cddb on Arcanemagus:rules-docs-url into 0d44914 on benmosher:master.

coveralls · 2018-01-09T02:52:20Z

Coverage increased (+0.01%) to 96.225% when pulling c05cddb on Arcanemagus:rules-docs-url into 0d44914 on benmosher:master.

ljharb requested changes Jan 8, 2018

View reviewed changes

Move URL generation to a function

3ea1050

Move the common string of the URL to a function that is called. Note that `imports-first` still has a fully defined URL in the rule file as the documentation for that rule requires a specific blob id since it has been deleted on `master`.

ljharb reviewed Jan 8, 2018

View reviewed changes

Arcanemagus added 3 commits January 8, 2018 14:18

Use commit hash for imports-first documentation link

fe03d72

Fix require of first in imports-first

f12b6d0

ljharb approved these changes Jan 8, 2018

View reviewed changes

ljharb added the docs label Jan 8, 2018

j-f1 reviewed Jan 8, 2018

View reviewed changes

Fix direct link to a file

c05cddb

The `/tree/` links are meant for directories, `/blob/` leads to an exact file. This allowed a simplification of the `docsUrl` function since the default branch can be treated as a "commit hash".

Arcanemagus mentioned this pull request Jan 8, 2018

Add URL to rule documentation to the metadata cvisco/eslint-plugin-requirejs#105

Merged

ljharb merged commit 1e980af into import-js:master Jan 9, 2018

Arcanemagus deleted the rules-docs-url branch January 9, 2018 06:45

renovate bot mentioned this pull request Feb 22, 2018

Update dependency eslint-plugin-import to v2.11.0 Urigo/tortilla#68

Merged

renovate bot mentioned this pull request Mar 22, 2018

Update eslint-plugin-import to 2.12.0 sbardian/DebtTrackerAPI#25

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add URL to rule documentation to the metadata #998

Add URL to rule documentation to the metadata #998

Arcanemagus commented Jan 8, 2018 •

edited

Loading

Arcanemagus commented Jan 8, 2018

ljharb left a comment

ljharb Jan 8, 2018

j-f1 Jan 8, 2018

Arcanemagus Jan 8, 2018 •

edited

Loading

Arcanemagus commented Jan 8, 2018

ljharb Jan 8, 2018

Arcanemagus Jan 8, 2018 •

edited

Loading

ljharb Jan 8, 2018

Arcanemagus Jan 8, 2018

Arcanemagus Jan 8, 2018

coveralls commented Jan 8, 2018 •

edited

Loading

coveralls commented Jan 8, 2018 •

edited

Loading

j-f1 Jan 8, 2018 •

edited

Loading

Arcanemagus Jan 8, 2018 •

edited

Loading

coveralls commented Jan 9, 2018 •

edited

Loading

coveralls commented Jan 9, 2018 •

edited

Loading

coveralls commented Jan 9, 2018

coveralls commented Jan 9, 2018 •

edited

Loading

coveralls commented Jan 9, 2018 •

edited

Loading

coveralls commented Jan 9, 2018 •

edited

Loading

		@@ -1,8 +1,12 @@
		import Exports from '../ExportMap'

		const ruleDocsUrl = 'https://github.com/benmosher/eslint-plugin-import/tree/master/docs/rules'

Add URL to rule documentation to the metadata #998

Add URL to rule documentation to the metadata #998

Conversation

Arcanemagus commented Jan 8, 2018 • edited Loading

Arcanemagus commented Jan 8, 2018

ljharb left a comment

Choose a reason for hiding this comment

ljharb Jan 8, 2018

Choose a reason for hiding this comment

j-f1 Jan 8, 2018

Choose a reason for hiding this comment

Arcanemagus Jan 8, 2018 • edited Loading

Choose a reason for hiding this comment

Arcanemagus commented Jan 8, 2018

ljharb Jan 8, 2018

Choose a reason for hiding this comment

Arcanemagus Jan 8, 2018 • edited Loading

Choose a reason for hiding this comment

ljharb Jan 8, 2018

Choose a reason for hiding this comment

Arcanemagus Jan 8, 2018

Choose a reason for hiding this comment

Arcanemagus Jan 8, 2018

Choose a reason for hiding this comment

coveralls commented Jan 8, 2018 • edited Loading

coveralls commented Jan 8, 2018 • edited Loading

j-f1 Jan 8, 2018 • edited Loading

Choose a reason for hiding this comment

Arcanemagus Jan 8, 2018 • edited Loading

Choose a reason for hiding this comment

coveralls commented Jan 9, 2018 • edited Loading

coveralls commented Jan 9, 2018 • edited Loading

coveralls commented Jan 9, 2018

coveralls commented Jan 9, 2018 • edited Loading

coveralls commented Jan 9, 2018 • edited Loading

coveralls commented Jan 9, 2018 • edited Loading

Arcanemagus commented Jan 8, 2018 •

edited

Loading

Arcanemagus Jan 8, 2018 •

edited

Loading

Arcanemagus Jan 8, 2018 •

edited

Loading

coveralls commented Jan 8, 2018 •

edited

Loading

coveralls commented Jan 8, 2018 •

edited

Loading

j-f1 Jan 8, 2018 •

edited

Loading

Arcanemagus Jan 8, 2018 •

edited

Loading

coveralls commented Jan 9, 2018 •

edited

Loading

coveralls commented Jan 9, 2018 •

edited

Loading

coveralls commented Jan 9, 2018 •

edited

Loading

coveralls commented Jan 9, 2018 •

edited

Loading

coveralls commented Jan 9, 2018 •

edited

Loading