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

Sharing #229

Merged
merged 12 commits into from
Sep 25, 2016
Merged

Sharing #229

merged 12 commits into from
Sep 25, 2016

Conversation

eliasmalik
Copy link
Contributor

@eliasmalik eliasmalik commented Sep 23, 2016

Changes necessary to make this config easily shareable.

Tested this locally using npm link and adding an .eslintrc.json file to a new project with the following contents:

{
  "extends": "dwyl"
}

Updated README with instructions.

All thats then left to be done is to publish to npm. Current version is 0.0.1 which will be the initial version unless someone has a problem with it.

Copy link
Member

@jrans jrans left a comment

Choose a reason for hiding this comment

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

Sorry to be annoying! Just small things but good job on the whole 👍

"peerDependencies": {
"eslint": ">=3.5.0"
},
"contributors": [
Copy link
Member

Choose a reason for hiding this comment

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

gitter handle may be better! hehe

Copy link
Member

Choose a reason for hiding this comment

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

@nelsonic included too!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to github handles and added @nelsonic

@@ -1,12 +1,23 @@
{
"name": "goodparts",
"name": "eslint-config-dwyl",
Copy link
Member

Choose a reason for hiding this comment

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

do we want to double check name with everyone? brings us back to #192 , the name of this module will be a good indication

Copy link
Member

Choose a reason for hiding this comment

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

dwyl-goodparts eslint-goodparts eslint-dwyl all possible contenders too

Copy link
Member

Choose a reason for hiding this comment

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

They all sound fine

"extends": "dwyl"
}
```
Alternatively, you can also add this to the `"eslintConfig"` field in your `package.json`.
Copy link
Member

Choose a reason for hiding this comment

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

I'm lazy can you provide the exact linting command

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made this explicit in 1e7ab4c

@jrans
Copy link
Member

jrans commented Sep 23, 2016

Before merging name of package needs to be decided but then ready, @Shouston3 @SimonLab @nelsonic thoughts?

@eliasmalik
Copy link
Contributor Author

eliasmalik commented Sep 23, 2016

After speaking to @nelsonic we decided we wanted a zero configuration setup, so I've added a simple bin file that will run the eslint CLI with our config.

Also there is already an npm package under the name goodparts, so I've gone back to that.

Then you can add the following script to your `package.json`:
```
{
"lint": "node_modules/.bin/goodparts path/to/files/for/linting"
Copy link
Member

Choose a reason for hiding this comment

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

@eliasCodes could we specify a bin script in package.json ?
see: https://docs.npmjs.com/files/package.json#bin

Copy link
Contributor Author

@eliasmalik eliasmalik Sep 23, 2016

Choose a reason for hiding this comment

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

I have 82796e6

Or do you mean changing the script to "lint": "goodparts path/to/files/for/linting"

Copy link
Member

Choose a reason for hiding this comment

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

Looks like a good start. thanks!

@nelsonic
Copy link
Member

thanks @eliasCodes !! 👍
❤️ ✅ 🚢

@nelsonic nelsonic merged commit 6377efb into master Sep 25, 2016
@nelsonic nelsonic deleted the sharing branch September 25, 2016 21:33
@iteles
Copy link
Member

iteles commented Sep 25, 2016

Did we go with goodparts just for now or did you guys actually make a call on the name?

@eliasmalik
Copy link
Contributor Author

@iteles we went with it for now because we own this already. If anyone feels strongly about the name we can open another discussion.

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.

6 participants