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: Distinguish examples in rules under Stylistic Issues part 7 #6760

Merged
merged 1 commit into from Aug 25, 2016

Conversation

scriptdaemon
Copy link
Contributor

What issue does this pull request address?

Documentation update.

What changes did you make? (Give an overview)

Documentation update.

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

  • one-var
    • Combined the object properties for each object option. Willing to reconsider if desired.
    • Added language for using one of three different options
  • quote-props
    • Do we have special language for object options that only apply based on choices of previous options? My only concern with this rule, I believe.

@pedrottimark Got another batch for ya.

@mention-bot
Copy link

@scriptdaemon, thanks for your PR! By analyzing the annotation information on this pull request, we identified @IanVS, @BYK and @pedrottimark to be potential reviewers

@eslintbot
Copy link

LGTM


## Options

There are two main options for the rule:
This rule has a string option:
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, this style is a bit confusing to me. The multiple "this rule has an X option" makes it difficult for me to understand if there's just one option that can be a string or object, or if there are multiple options.

I'd much rather we start with "This rule has two options," as I think that's a lot clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What if we added language like so:

This rule has a string option:

*Stuff*

And an object option:

*More stuff*

This would make the language consistent with other forms that we currently use, such as:

This rule has a string option:

*Stuff*

Or an object option:

*More stuff*

Copy link
Member

Choose a reason for hiding this comment

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

Why can't we say, "This rule has two options, a string option and an object option"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not condense that to "This rule has a string option and an object option" so that it's consistent and makes more sense to say "This rule has two string options" when the options are of the same type? Seems @pedrottimark has been busy, but I'd like to know what he thinks as well since the guidelines would need to be updated.

Copy link
Member

Choose a reason for hiding this comment

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

@pedrottimark Any suggestions?

Copy link
Member

Choose a reason for hiding this comment

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

@scriptdaemon As I said, I don't believe it's clear enough. For example "it has a string option and an object option" in one rule and "it has a string option OR an object option" in another. That's a really subtle difference for a reader to pick up, especially if English isn't their first language.

My rule of thumb is that if there's a choice between being explicit or not, always choose being explicit. That would change my examples to "This rule has two options, a string option and an object option" and then "This rule has one option, which can be a string option or an object option." Both of which leave little confusion over what is being described.

If there are other rule docs that aren't including the rule option count, then I apologize for missing those as they went through. I think we need to make this the convention going forward.

@eslintbot
Copy link

LGTM

@scriptdaemon
Copy link
Contributor Author

@nzakas Now that you put it that way, I see your reasoning. I updated the PR. What do you think of these changes? I separated the text "This rule has two options, a string option:" "And an object option:" to be above their respective options. Should we keep that format, or do you have another suggestion?

Also, I'll add these to the guidelines later. Quite a few docs got through with the old style, however.

@platinumazure
Copy link
Member

Speaking just for myself, when I get to "Or an object option:", I will probably have lost the context by then. I'd much rather see the full sentence ("This rule has one option, which may be a string object or an object option.") and then section delimiters ("String options:" or similar, and "Object option:" or similar).

@eslintbot
Copy link

LGTM

@scriptdaemon
Copy link
Contributor Author

@platinumazure Updated with that in mind. What do you think of how it reads now?

@platinumazure
Copy link
Member

I like it a lot more now. Hopefully @nzakas will agree.

@ilyavolodin
Copy link
Member

LGTM, but waiting another day for others to look

@gyandeeps
Copy link
Member

LGTM 👍
Thanks for contributing.

@gyandeeps gyandeeps merged commit 2dfb290 into eslint:master Aug 25, 2016
@scriptdaemon scriptdaemon deleted the docs-stylistic-issues-7 branch August 25, 2016 02:07
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants