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

Add a style guide #165

Open
PirosB3 opened this issue Sep 23, 2018 · 5 comments
Open

Add a style guide #165

PirosB3 opened this issue Sep 23, 2018 · 5 comments

Comments

@PirosB3
Copy link
Contributor

PirosB3 commented Sep 23, 2018

It would be great to define a specific style guide to enforce. This would make sure we all have the same expectation when writing code, and we can also take advantage of automated tooling for lint. It looks like Airbnb, GitHub, & Google all have some open source style guide that we can take advantage of, but I don't have experience with any of them.

Would you be interested in enforcing a style guide for the next contributions? if so, do you have a preference?

@mably
Copy link
Owner

mably commented Sep 23, 2018

Yep definitely, Airbnb seems interesting to me.

@PirosB3
Copy link
Contributor Author

PirosB3 commented Sep 23, 2018

Sounds great!

@PirosB3
Copy link
Contributor Author

PirosB3 commented Sep 28, 2018

Sounds great! So I have made a branch containing the tooling and stile documentation using Airbnb JS (https://github.com/PirosB3/lncli-web/tree/lnd-manager-linting). In order to avoid confusion with #163, I will wait till that PR gets merged to master and then I will open a subsequent PR containing style fixes

@mably
Copy link
Owner

mably commented Sep 28, 2018

FYI we are already using jscs for code validation (via `gulp'), let's try not to break everything and be as close as possible as what is already in place, unless it can be improved of course.

@PirosB3
Copy link
Contributor Author

PirosB3 commented Sep 28, 2018

Thanks for pointing that out, I actually didn't know that! said this, It looks like JSCS has merged with ESLint (https://www.npmjs.com/package/jscs) and ESLint contains default for Airbnb guide. JSCS website is also gone lol.

I'm opening my PR now so you can take a look at the changes I did, and we can chat more about this on the PR

This was referenced Sep 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants