gitcoinco / web

Grow Open Source
https://gitcoin.co
Other
1.78k stars 771 forks source link

core : contributing.md guideline + linter #65

Closed thelostone-mc closed 6 years ago

thelostone-mc commented 6 years ago

Folks have different editor setting based on personal preference & since gitcoin looks promising maybe we should consider having a style guide to ensure everyone follows a particular design style Example:

Reason I suggest this: I had changed a line in css and git diff showed 18 changes ( 17 were removal of trailing whitespace )

owocki commented 6 years ago

i was talking to someone on slack the other day who suggested this too. idea must be in the 'ether' right now so to speak!

im game to

beyond that, should we adopt the pep8 standard? what are all the cool kids doing these days?

thelostone-mc commented 6 years ago

Few folks in my circle ( not sure if we could label ourselves cool just yet ) use the google python style guide but pep8 still stands tall. It's recommended by folks at django. We could always update it suit our needs like maybe avoid breaking lines at 79 chars to make code look less clunkier!

CSS/SCSS (the options are plenty!)

As long as it's uniform everyone is happy. ^_^

gamwe6 commented 6 years ago

When I saw this issue I thought "but there is already a CONTRIBUTING.md file", but when I checked the repo ... nothing ... I forgot to push it XD. If you want to do it @thelostone-mc go ahead!

thelostone-mc commented 6 years ago

I'll get to it, if @owocki doesn't already have one ready ^_^

owocki commented 6 years ago

go for it @thelostone-mc !

btw, send me a message on slack pls. have a quick question for you

ethikz commented 6 years ago

I'll implement a linter and fix all lint issues for scss. After that I'll add in eslint.

Or if you want to handle scss lint @thelostone-mc I can do the js lint

thelostone-mc commented 6 years ago

@ethikz I've sort of added a css linter. Could you take of the JS lint bit ?

ethikz commented 6 years ago

@thelostone-mc haha just updated my comment to make that change 😄 .

I also have a little more stringent scss_lint if you would like to compare

thelostone-mc commented 6 years ago

@ethikz Yup def comparing ^_^ PS: are we planning on using ESlint or something else ?

ethikz commented 6 years ago

Yes I was planning on it. This way the community is up to date(ish). Unless you have a reason it might not be a good idea?

I was thinking we could go ahead and start using SCSS and es6

thelostone-mc commented 6 years ago

Nope I'm all for it !! :D

ethikz commented 6 years ago

Also I've uploaded the config I've used in multiple projects.

https://github.com/ethikz/guidelines

ethikz commented 6 years ago

@thelostone-mc How are you implementing the linter? Node/npm? Gulp? Grunt?

Trying to figure out what I should do to implement the eslint if I'm going to use webpack or what.

thelostone-mc commented 6 years ago

@ethikz I was thinking node + gulp if that's alright! You've got something else in mind ? I'm a little new to docker, so just getting my head around everything

ethikz commented 6 years ago

@thelostone-mc Well I wasn't going to add to docker just yet. I was thinking since Webpack can handle the linting I would just do npm scripts so we don't have the dependency of gulp and other packages it needs. I'm open though, whichever is easiest.

thelostone-mc commented 6 years ago

@ethikz let's do that then ^_^

ethikz commented 6 years ago

Sounds good

mbeacom commented 6 years ago

👍 If you need any assistance with the docker part, let me know! Realistically, we should consider adding a pre-commit hook for linting (and isort/flake8). I've intentionally avoided pep8ing the backend code for the moment.

ethikz commented 6 years ago

Yea I think that is a good idea although I think having the linting also on the build steps would be helpful or giving the people the ability to run them before hand so they don't have to keep committing fixes to code and can do it in one go.

thelostone-mc commented 6 years ago

@mbeacom I'll DM you with a bunch a questions ^_^

Elaniobro commented 6 years ago

I would strongly recommend stylelint.io which supports css rules for:

To allow everyone to use their own editors, I would also highly recommend that you include the .editorconfig here is an example of one I use at work:

root = true

[*]
indent_style = space
indent_size = 4
end_of_line = lf
charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = true

[{package.json,*.yml}]
indent_style = space
indent_size = 2

[*.md]
trim_trailing_whitespace = false
thelostone-mc commented 6 years ago

@Elaniobro stylelint was my choice too ^_^ +1 for the .editorconfig

thelostone-mc commented 6 years ago

@Elaniobro @ethikz : I was planning on using stylelint-config-standard out of the box.

I've gotten a lot of good feedback from this. Figured we could use this and change it along the way if needed

owocki commented 6 years ago

once the holidays are over, we should bump this back and close it down!