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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs: Distinguish examples in rules under Stylistic Issues part 6 #6567

Merged
merged 1 commit into from Jul 2, 2016

Conversation

scriptdaemon
Copy link
Contributor

Found some time to get a quick batch in.

object-curly-newline has the option to have separate configurations for object literals and destructuring assignments. My mind is drawing a blank, did we have any specific way to handle options for this kind of case?

@pedrottimark Mind giving this a look?

Slowly and surely... 馃槂

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @mysticatea, @pedrottimark and @cjihrig to be potential reviewers

@eslintbot
Copy link

LGTM


## Rule Details

The following patterns are considered problems:
This rule disallow trailing whitespace at the end of lines.
Copy link
Member

Choose a reason for hiding this comment

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

s/disallow/disallows/

Copy link
Member

Choose a reason for hiding this comment

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

What do you think about deleting the comment in incorrect code below and adding it here?

This rule disallows trailing whitespace (spaces, tabs, and other Unicode whitespace characters) at the end of lines.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, sorry for making the second suggestion in an ambiguous way. We ended up with two similar sentences in the paragraph.

@pedrottimark
Copy link
Member

pedrottimark commented Jun 30, 2016

@scriptdaemon Thank you. Nice work. 5 rules was a good amount for now.

In object-curly-newline

  • I suggested some sentences. According to my notes, you will see more of this in the next few batches: one-var, padded-blocks, space-before-function-paren so we can learn as we go.
  • Can you delete the hyphens in the list items under Options so they read like simple sentences
  • Just to confirm the reasoning (which makes sense to me) you deleted the multiline and minProperties section because "multiline": true is the default option, [EDIT] therefore that example code is redundant with (and almost but not quite identical to) the example code in the preceding minProperties section?

}
} = obj;
```

### separating configuration
Copy link
Member

Choose a reason for hiding this comment

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

An alternative to consider for this heading is ObjectExpression and ObjectPattern

@eslintbot
Copy link

LGTM

@scriptdaemon
Copy link
Contributor Author

@pedrottimark Thank you. Yeah, I figured 5 was better than none.

The examples for combined multiline and minProperties did seem redundant to me, so that is why I removed them.

@pedrottimark
Copy link
Member

Only one more change in no-trailing-spaces where my line comment was ambiguous.

@eslintbot
Copy link

LGTM

@scriptdaemon
Copy link
Contributor Author

@pedrottimark Fixed. Probably should have paid more attention when making the latest updates. 馃槃

@pedrottimark
Copy link
Member

Looks good to me

@ilyavolodin ilyavolodin merged commit f6b8452 into eslint:master Jul 2, 2016
@scriptdaemon scriptdaemon deleted the docs-stylistic-issues-6 branch July 3, 2016 05:56
@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

6 participants