Add URL to rule documentation to the metadata #141

Arcanemagus · 2018-01-08T23:53:41Z

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.

SamVerschueren · 2018-01-09T00:10:43Z

I believe it would be a lot more maintainable if we created a utility function called getDocsURL() which uses module.parent.filename to dynamically calculate the last bit of the URL. All the other stuff is static.

Add a utility function `getDocsUrl` in order to determine what the URL of the rule should be. Although not currently used here this function is written to support an optional second paramater of the commit hash to link the documentation to, in case this is needed later.

Arcanemagus · 2018-01-09T05:09:06Z

Moved to that model @SamVerschueren 😉. Since I didn't see any existing place to put this function I created a util folder in the rules folder. I also added some basic test for this new utility function.

SamVerschueren

I would make ruleName optional as well and extract from module.parent.filename inside the utility function.

SamVerschueren · 2018-01-09T06:16:25Z

rules/utils/get-docs-url.js

+
+const repoUrl = 'https://github.com/sindresorhus/eslint-plugin-unicorn';
+
+function getDocsUrl(ruleName, givenCommitHash) {


Export this one directly.

Rename givenCommitMash to commitHash. Below, write it like this

commitHash = commitHash || 'master';

Export this one directly.

The rest of this was written in CommonJS, so that's the style I was following. I'll switch it to a module 👍.

Rename givenCommitHash to commitHash.

It's generally a bad idea to assign to parameters, but I can change it to that style.

I meant module.exports = (ruleName, commit:ash) => {

Strings are immutable. It's just the same as a default value, only one line extra. It's a bad idea when an object is passed in and you'd write obj.commitHash = obj.commitHash || 'master'

SamVerschueren · 2018-01-09T06:16:51Z

rules/utils/get-docs-url.js

@@ -0,0 +1,13 @@
+'use strict';
+
+const repoUrl = 'https://github.com/sindresorhus/eslint-plugin-unicorn';


Move it to the top

Use `module.parent.filename` to automatically determine the rule name if a specific one is not passed in.

Arcanemagus · 2018-01-09T07:13:08Z

Is that more along the lines of what you were thinking @SamVerschueren?

SamVerschueren

Some minor improvements. Looks good overall! Reviewing on mobile so might be missing some other small things :).

SamVerschueren · 2018-01-09T07:38:57Z

rules/utils/get-docs-url.js

+
+const repoUrl = 'https://github.com/sindresorhus/eslint-plugin-unicorn';
+
+module.exports = function (ruleName, commitHash) {


Use an arrow function

SamVerschueren · 2018-01-09T07:39:37Z

rules/utils/get-docs-url.js

@@ -0,0 +1,11 @@
+'use strict';
+const {basename} = require('path');


Use const path = require('path')

SamVerschueren · 2018-01-09T07:40:59Z

rules/utils/get-docs-url.js

+const repoUrl = 'https://github.com/sindresorhus/eslint-plugin-unicorn';
+
+module.exports = function (ruleName, commitHash) {
+	ruleName = ruleName || basename(module.parent.filename).replace('.js', '');


Just pass .js as second argument to path.basename(). See https://nodejs.org/api/path.html#path_path_basename_path_ext

SamVerschueren · 2018-01-09T07:41:25Z

test/get-docs-url.js

+import getDocsUrl from '../rules/utils/get-docs-url';
+
+test('returns the URL of the a named rule\'s documentation', t => {
+	t.plan(1);


SamVerschueren · 2018-01-09T07:41:34Z

test/get-docs-url.js

+});
+
+test('returns the URL of the a named rule\'s documentation at a commit hash', t => {
+	t.plan(1);


SamVerschueren · 2018-01-09T07:41:43Z

test/get-docs-url.js

+});
+
+test('determines the rule name from the file', t => {
+	t.plan(1);


Arcanemagus · 2018-01-09T16:56:26Z

Updated again @SamVerschueren 😉.

SamVerschueren

One minor comment but looks good to me. One thing I thought about as well, but need a second opinion from @sindresorhus, is that we could also write docs: getDocs() which returns the entire docs object instead of what we have now.

SamVerschueren · 2018-01-09T17:36:53Z

rules/utils/get-docs-url.js

+
+const repoUrl = 'https://github.com/sindresorhus/eslint-plugin-unicorn';
+
+module.exports = (ruleName, commitHash) => {


Not sure about the commit hash as we don't use it currently.

It's used in a few other plugins that I sent PRs to, so I figured I would leave it in here. I can remove it if you want though.

I think we should remove it as long as we don't need it. It adds "complexity" and should be maintained later on if perhaps the function gets more complex. But that's up to Sindre to decide.

sindresorhus · 2018-01-14T22:32:54Z

Thanks for adding this @Arcanemagus :)

SamVerschueren suggested changes Jan 9, 2018

View reviewed changes

Determine rule name automatically

23d5f8f

Use `module.parent.filename` to automatically determine the rule name if a specific one is not passed in.

SamVerschueren suggested changes Jan 9, 2018

View reviewed changes

🎨 Address feedback

9b35aa3

SamVerschueren approved these changes Jan 9, 2018

View reviewed changes

sindresorhus added 3 commits January 14, 2018 23:17

Update get-docs-url.js

f97becd

Update get-docs-url.js

1a29f9e

Update get-docs-url.js

5a5e8ac

sindresorhus merged commit 875344d into sindresorhus:master Jan 14, 2018

Arcanemagus deleted the rules-docs-url branch January 15, 2018 09:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add URL to rule documentation to the metadata #141

Add URL to rule documentation to the metadata #141

Arcanemagus commented Jan 8, 2018

SamVerschueren commented Jan 9, 2018

Arcanemagus commented Jan 9, 2018 •

edited

SamVerschueren left a comment

SamVerschueren Jan 9, 2018

SamVerschueren Jan 9, 2018

Arcanemagus Jan 9, 2018

SamVerschueren Jan 9, 2018

SamVerschueren Jan 9, 2018

SamVerschueren Jan 9, 2018

Arcanemagus commented Jan 9, 2018

SamVerschueren left a comment

SamVerschueren Jan 9, 2018

SamVerschueren Jan 9, 2018

SamVerschueren Jan 9, 2018

SamVerschueren Jan 9, 2018

SamVerschueren Jan 9, 2018

SamVerschueren Jan 9, 2018

Arcanemagus commented Jan 9, 2018

SamVerschueren left a comment

SamVerschueren Jan 9, 2018

Arcanemagus Jan 9, 2018

SamVerschueren Jan 9, 2018

sindresorhus commented Jan 14, 2018


		const repoUrl = 'https://github.com/sindresorhus/eslint-plugin-unicorn';

		function getDocsUrl(ruleName, givenCommitHash) {

		@@ -0,0 +1,13 @@
		'use strict';

		const repoUrl = 'https://github.com/sindresorhus/eslint-plugin-unicorn';


		const repoUrl = 'https://github.com/sindresorhus/eslint-plugin-unicorn';

		module.exports = function (ruleName, commitHash) {

		@@ -0,0 +1,11 @@
		'use strict';
		const {basename} = require('path');


		const repoUrl = 'https://github.com/sindresorhus/eslint-plugin-unicorn';

		module.exports = (ruleName, commitHash) => {

Add URL to rule documentation to the metadata #141

Add URL to rule documentation to the metadata #141

Conversation

Arcanemagus commented Jan 8, 2018

SamVerschueren commented Jan 9, 2018

Arcanemagus commented Jan 9, 2018 • edited

SamVerschueren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Arcanemagus commented Jan 9, 2018

SamVerschueren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Arcanemagus commented Jan 9, 2018

SamVerschueren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sindresorhus commented Jan 14, 2018

Arcanemagus commented Jan 9, 2018 •

edited