Add travis ci#359
Conversation
|
If you like to upgrade the node requirement, please also modify this line: |
|
@jackycute Updated. |
|
Thanks @bananaappletw , |
|
@a60814billy |
a60814billy
left a comment
a60814billy
left a comment
There was a problem hiding this comment.
- Consider use
maketo run eslint or not - modify README.md, test version
| @@ -0,0 +1,3 @@ | |||
| lint: | |||
| @./node_modules/.bin/eslint . | |||
There was a problem hiding this comment.
Maybe, the script can move to package.json?
There was a problem hiding this comment.
@a60814billy For further purpose, we should use Makefile to write test.
It make clearier to write differenct test in Makefile.
Refer: https://github.com/koajs/koa/blob/master/Makefile
There was a problem hiding this comment.
How about combine this to webpack? since we already using that.
Seems like only koa use Makefile, maybe it's using Makefile for some other purpose than just lint or test.
I start thinking this is just a preference problem.
There was a problem hiding this comment.
And tj himself seems like stop using makefile to run npm scripts in his recent (in an year) repos 😢.
There was a problem hiding this comment.
@jackycute Ok, I try to use webpack instead.
Thanks.
| --- | ||
|
|
||
| - Node.js 4.x or up (test up to 6.7.0) | ||
| - Node.js 6.x or up (test up to 6.7.0) |
There was a problem hiding this comment.
In travis, already use latest node 7 version to test.
Please modify this line to node 7 latest version.
There was a problem hiding this comment.
I think as long as HackMD says it supports node version 6 we should test it. As tests are made to validate possible problems and not have a magic "it should work" somewhere.
There was a problem hiding this comment.
Yeah, we'll test the under node 6 version.
Above comment just saying that the "test up to" should be node 7.
There was a problem hiding this comment.
@a60814billy I will modify this.
Thanks.
| indent_style = space | ||
| indent_size = 2 | ||
| [Makefile] | ||
| indent_style = tab |
There was a problem hiding this comment.
Mixing up intent styles (even for other file types) is worst you can do and shouldn't be done!
There was a problem hiding this comment.
I'm not sure about this.
I thought different code language might have their best indent that might vary.
Would you explain your intend?
There was a problem hiding this comment.
I know the importance of consistency.
@bananaappletw what do you think?
There was a problem hiding this comment.
@SISheogorath @jackycute Tab character has special meaning for Makefile.
So it is correct to use tab indent instead of space in Makefile.
See here
Different programming language has its best intent, agree with that.
There was a problem hiding this comment.
I personally program most of the time in vim. And the worst thing you can have there is different intent style, than you have in your .vimrc.
In general speaking, you try to find a standard which says tabs or spaces. and also the intent size. Not only for you, for the whole company/project. And as long as you do not configure this in a central config file or a per file config (like possible in vim), you should stay with one type because otherwise you need to create a special config file, per project, per filetype which is more than ugly...
There was a problem hiding this comment.
@SISheogorath Maybe you have to take a look of this
Tab character has special meaning in Makefile
There was a problem hiding this comment.
@SISheogorath Thanks for telling more.
But I think it won't apply to Makefile, because "a tab" indent is part for the format.
It might fail to compile if you don't start line with a tab sometimes (true story).
There was a problem hiding this comment.
Yes, I was again misleading myself :D
Thought you edit another file. It's okay :D
There was a problem hiding this comment.
Well, it is possible to use another character than tabs... but not really useful.
See .RECIPEPREFIX
| indent_style = space | ||
| indent_size = 2 | ||
| [Makefile] | ||
| indent_style = tab |
There was a problem hiding this comment.
Well, it is possible to use another character than tabs... but not really useful.
See .RECIPEPREFIX
|
Mhm, someone has to run |
|
@SISheogorath yeah, so this PR is more like a pre-setup one. |
Exclude *.min.js from eslint
|
I change to use package.json to test lint. |
|
Thanks @bananaappletw , |
|
Let's merge this first, then change the rules when needed. |
4 participants
Add eslint syntax check by travis ci service