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: Add linting to rule doc example code blocks (1 of 3) (refs #2271) #3487

Merged
merged 1 commit into from Aug 28, 2015

Conversation

IanVS
Copy link
Member

@IanVS IanVS commented Aug 22, 2015

This is a work in progress, and will take some time to complete. However, if anyone has feedback on this, please speak up now before I go through all 170-something rules. :)

One specific item: some of the lines get pretty long with these expected errors. I'm not sure how that will look on the eslint.org site. Is there a way to check that?

@ilyavolodin
Copy link
Member

Nice! I like it.

@BYK
Copy link
Member

BYK commented Aug 23, 2015

😲 👍 🎉

@IanVS
Copy link
Member Author

IanVS commented Aug 25, 2015

I had to turn off linting for rule linebreak-style, which I did by removing the js info string from the code fences. This is because the \r\n and \n are being treated as code, and not linebreaks.

@btmills
Copy link
Member

btmills commented Aug 25, 2015

Would moving the line endings into informational comments work? It wouldn't change the meaning of the examples, but it would make them valid JS.

@IanVS
Copy link
Member Author

IanVS commented Aug 25, 2015

@btmills, I like that idea. That's what I'll do.

@nzakas
Copy link
Member

nzakas commented Aug 25, 2015

Just a heads up - the longer this sits, the harder it will be to merge. :)

@IanVS
Copy link
Member Author

IanVS commented Aug 25, 2015

I understand, but I'm not just sitting on it. I've been spending several hours a night on it, and am maybe 30% done at this point. 😞

I've been rebasing periodically, and so far it doesn't have any problems. I'll give it a good once-over when I'm complete to make sure I've not missed anything that's been added. I'm hoping to have it done within another week or two.

@IanVS
Copy link
Member Author

IanVS commented Aug 26, 2015

A bit of a snag: some rule examples require ecmaFeatures: { modules: true } and others require modules: false. Until a better solution can be found (maybe involving globbing), I'm removing the info string from the rules which require modules to be turned off. These are:

@IanVS
Copy link
Member Author

IanVS commented Aug 26, 2015

I followed @btmills advice for linebreak-style, and also added the actual CRLF line endings as appropriate, so that linting works correctly. To do so, I also needed to add a .gitattributes to prevent automatic conversion on that file.

@nzakas
Copy link
Member

nzakas commented Aug 26, 2015

No worries, just wanted to flag that. You might want to consider ways of doing partial commits to limit the merge conflict likelihood. "Big bang" commits are almost always a bad idea. :)

@IanVS
Copy link
Member Author

IanVS commented Aug 26, 2015

OK. I'm happy to split into a few commits if that's an option. It would actually be good to at least see how this looks in some rules on the site, too. I'm about half-way through, so I'll squash and rebase with just the rule changes so as not to break the build. After that's merged, I'll finish up the rule changes and finally add the changes to makefile.js

@ilyavolodin
Copy link
Member

@IanVS You can take a look at #397 that's where I migrated all unittests, so had to go through the same trouble as you are going through right now.

@IanVS
Copy link
Member Author

IanVS commented Aug 27, 2015

OK, tracking in: #2271 (comment)

@IanVS IanVS changed the title WIP: Add linting to rule doc example code blocks (fixes #2271) Docs: Add linting to rule doc example code blocks (1 of 3) (refs #2271) Aug 27, 2015
@IanVS IanVS force-pushed the fix_2271 branch 2 times, most recently from 0be0a8a to a952fd9 Compare August 28, 2015 01:50
@@ -6,3 +6,10 @@ indent_size = 4
trim_trailing_whitespace = true
end_of_line = lf
insert_final_newline = true

Copy link
Member

Choose a reason for hiding this comment

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

Nice touch!

@ilyavolodin
Copy link
Member

LGTM for the first batch

ilyavolodin added a commit that referenced this pull request Aug 28, 2015
Docs: Add linting to rule doc example code blocks (1 of 3) (refs #2271)
@ilyavolodin ilyavolodin merged commit a7f6b67 into master Aug 28, 2015
@ilyavolodin ilyavolodin deleted the fix_2271 branch August 28, 2015 02:35
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 7, 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 7, 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