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

Enforce consistent indentation style #10

Merged
merged 2 commits into from
Sep 17, 2016

Conversation

gajus
Copy link
Contributor

@gajus gajus commented Sep 17, 2016

#9 introduced tabs. I have noticed only post merge. This PR fixes the style issue introduced by the #9 and adds .editorconfig to prevent future mistakes such as this one.

@gajus gajus mentioned this pull request Sep 17, 2016
@stubailo
Copy link
Contributor

Hmm, we probably want to set up linting then. I just published a new version, perhaps you can rebase this PR?

@gajus
Copy link
Contributor Author

gajus commented Sep 17, 2016

Hmm, we probably want to set up linting then. I just published a new version, perhaps you can rebase this PR?

You most certainly should...

Ideally test and lint should be done precommit, e.g. gajus/react-css-modules@f806b20.

@stubailo
Copy link
Contributor

This package is super small, so it doesn't seem like it needs that much structure. I'd definitely be open to having linting though just to avoid stuff like tabs/spaces.

Having tests run pre-commit sounds pretty rough since that would make intermediate commits more onerous since you need to either skip the hook or make all tests pass.

@gajus
Copy link
Contributor Author

gajus commented Sep 17, 2016

Having tests run pre-commit sounds pretty rough since that would make intermediate commits more onerous since you need to either skip the hook or make all tests pass.

I had more or less the same argument (gajus/react-css-modules#174) with @lbeschastny. I was wrong.

Having a precommit hook for test/lint most definitely saves time and makes commit history cleaner.

@gajus
Copy link
Contributor Author

gajus commented Sep 17, 2016

I'd definitely be open to having linting though just to avoid stuff like tabs/spaces.

Just use https://github.com/airbnb/babel-preset-airbnb.

Or if you prefer biased and strict rules, https://github.com/gajus/eslint-config-canonical.

@stubailo
Copy link
Contributor

I'm open to either - usually I use airbnb with one or two custom rules but I'm not super opinionated as long as it's consistent.

@stubailo
Copy link
Contributor

Thanks for the spacing fix! I don't think we need to ship a new version, but it's much better now.

@stubailo stubailo merged commit 017be3d into apollographql:master Sep 17, 2016
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

Successfully merging this pull request may close these issues.

2 participants