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

Added stylelint configuration. #842

Open
wants to merge 35 commits into
base: develop
Choose a base branch
from

Conversation

gucong3000
Copy link
Contributor

@gucong3000 gucong3000 commented Aug 7, 2017

blake-newman and others added 10 commits June 28, 2016 16:43
# Conflicts:
#	docs/structure.md
#	meta.js
#	template/.eslintrc.js
#	template/build/webpack.dev.conf.js
#	template/package.json
#	template/src/App.vue
#	template/src/components/Hello.vue
#	template/src/main.js
# Conflicts:
#	template/.eslintrc.js
docs/linter.md Outdated
"indentation": "tab",
```

2. Pick "none" for ESLint preset when generating the project and define your own rules. See [Stylelint documentation](http://stylelint.io/user-guide/rules/) for more details.
Copy link
Contributor

Choose a reason for hiding this comment

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

ESLint => stylelint

Copy link
Contributor

@aladdin-add aladdin-add left a comment

Choose a reason for hiding this comment

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

almost LGTM.

@@ -85,6 +90,13 @@
{{/e2e}}
"semver": "^5.3.0",
"shelljs": "^0.7.6",
{{#stylelint}}
"stylelint": "github:stylelint/stylelint",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure there is a necessary to use github? it has no version control..I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm waiting for the next release of stylelint that including stylelint/stylelint#2799.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

stylelint/stylelint#2838
It's going to be released in couple of days.

meta.js Outdated
"stylelintConfig": {
"when": "stylelint",
"type": "list",
"message": "Pick an stylelint preset",
Copy link
Contributor

Choose a reason for hiding this comment

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

an => a. :(

@gucong3000 gucong3000 changed the title [WIP] Added stylelint configuration. Added stylelint configuration. Sep 5, 2017
@gucong3000
Copy link
Contributor Author

gucong3000 commented Sep 5, 2017

New bug of stylelint v8.1.0 block this PR: stylelint/stylelint#2853

@gucong3000 gucong3000 changed the title Added stylelint configuration. [WIP] Added stylelint configuration. Sep 12, 2017
@aladdin-add
Copy link
Contributor

ready to go?

@gucong3000
Copy link
Contributor Author

ready to go?

Not yet.
Another new bug in stylelint v8.1.0 (stylelint/stylelint#2853)
package.json use git url to install stylelint, we'd better wait for stylelint release next new virsion.

docs/linter.md Outdated
@@ -14,3 +15,16 @@ If you are not happy with the default linting rules, you have several options:
2. Pick a different ESLint preset when generating the project, for example [eslint-config-airbnb](https://github.com/airbnb/javascript/tree/master/packages/eslint-config-airbnb).

3. Pick "none" for ESLint preset when generating the project and define your own rules. See [ESLint documentation](http://eslint.org/docs/rules/) for more details.

## stylelint
This boilerplate uses [stylelint](http://stylint.io/) as for style linting, and uses the [Standard](https://github.com/stylelint/stylelint-config-standard) preset.
Copy link

@KarboniteKream KarboniteKream Sep 16, 2017

Choose a reason for hiding this comment

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

You have a typo in the stylelint URL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you.
fixed.

@exarus
Copy link
Contributor

exarus commented Oct 7, 2017

Stylelint v8.2.0 is out.

@gucong3000
Copy link
Contributor Author

gucong3000 commented Dec 3, 2017

but I'm worried that the circleCI tests fail, it seems the vm runs out of memory.

@LinusBorg
I need some help, I can't find the reason for this problem, the master branch has the same problem.
Could you please try to push this branch to your code base?

@LinusBorg
Copy link
Contributor

LinusBorg commented Dec 3, 2017

Okay, I'll push this as a separate branch to our repo and we'll see from there.

@gucong3000
Copy link
Contributor Author

@LinusBorg #1138

circle.yml Outdated
@@ -11,3 +11,9 @@ dependencies:
test:
override:
- bash test.sh
post:
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @gucong3000 Can you explain to me what this post hook does? I'm horrbile with bash.

Copy link
Contributor Author

@gucong3000 gucong3000 Dec 6, 2017

Choose a reason for hiding this comment

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

Move files from ./test/**/* to here:
image

Then we can download them.

@LinusBorg
Copy link
Contributor

@gucong3000 I want to hold off with this PR until #1133 is merged, is that ok?

@gucong3000
Copy link
Contributor Author

gucong3000 commented Dec 7, 2017

@LinusBorg Yes, of course.

@maxmilton
Copy link

@gucong3000 love the progress you've made on this so far, however, right now it's broken. I cloned the stylelint feature branch of your repo and initialised a new project but it's not working as expected.

Running yarn run lint:stylelint works correctly and linting errors are displayed.

Running yarn run dev compiles but does not show any errors.

I spent quite a bit of time on this in my own project and I havn't been able to debug the cause. It's definitely caused by some dependency but I couldn't figure out which. Have you experience this in your projects?

@gucong3000
Copy link
Contributor Author

gucong3000 commented Dec 18, 2017

@maxmilton Thank you for the report.

Running yarn run dev compiles but does not show any errors.

It's been repaired here.

@maxmilton
Copy link

maxmilton commented Dec 20, 2017

Just tested and everything works great. Thank you :)

Looking forward to seeing this as part of the template!

@gucong3000
Copy link
Contributor Author

😢

@gucong3000 gucong3000 force-pushed the feature/stylint branch 2 times, most recently from ecde369 to 020b0ea Compare January 19, 2018 07:12
@LinusBorg
Copy link
Contributor

I know, I know :(

All I can say is thank you for being that patient with me so far...

These bigger PRs are always a bit problematic for me in terms of finding time to properly testing them.

I'm also totally unfamiliar it stylelint, so will have to learn the basics to feel comfort able maintaining it on this template.

I will merge this and I won't take a moth t month to do so.

@gucong3000 gucong3000 force-pushed the feature/stylint branch 3 times, most recently from d4f082d to 709d922 Compare January 19, 2018 07:28
@gucong3000
Copy link
Contributor Author

Related: stylelint/stylelint#3148

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants