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

Closes #485 #495

Closed
wants to merge 3 commits into from
Closed

Closes #485 #495

wants to merge 3 commits into from

Conversation

@genediazjr
Copy link
Contributor

genediazjr commented May 27, 2016

updated to include gitignores

@gergoerdosi

This comment has been minimized.

Copy link
Contributor

gergoerdosi commented May 27, 2016

@genediazjr

This comment has been minimized.

Copy link
Contributor Author

genediazjr commented May 30, 2016

@arb is this ok?

@arb arb self-assigned this May 30, 2016
@arb arb added this to the 7.1.0 milestone May 30, 2016
@arb

This comment has been minimized.

Copy link
Contributor

arb commented May 30, 2016

Might as well add tests, 'API.md' and 'CONTRIBUTING.md' to that too.

Check out how joi did it here. So that the stuff in .gitignore is automatically kept in sync. Let's follow suit here.

@vdeturckheim

This comment has been minimized.

Copy link

vdeturckheim commented May 31, 2016

@arb regarding API.md: I like the fact to get packages from npm with their documentation.(Sometimes, I dev offline and I happen to go through 'node_modules' to find doc)

@genediazjr

This comment has been minimized.

Copy link
Contributor Author

genediazjr commented May 31, 2016

I agree with @vdeturckheim. what do you think @arb?

},
"scripts": {
"test": "lab -m 5000 -t 100 -v -La code",
"test-cov-html": "lab -m 5000 -r html -o coverage.html -La code"
"test-cov-html": "lab -m 5000 -r html -o coverage.html -La code",
"update-npmignore": "npmignore -i examples,images,test,API.md,CONTRIBUTING.md"

This comment has been minimized.

Copy link
@arb

arb Jun 5, 2016

Contributor

Do you need these -i arguments? I noticed Joi isn't doing that and that's what I'm basing this off of.

This comment has been minimized.

Copy link
@genediazjr

genediazjr Jun 5, 2016

Author Contributor

Yup. I tried running the joi v8 branch sample you have shared but it doesn't work. Only the gitignore content is added. So I followed the the API instead.

This comment has been minimized.

Copy link
@arb

arb Jun 7, 2016

Contributor

Sorry to be annoying, but I think I changed my mind :-) I'll just keep them in sync. It rarely, if EVER changes. So we can drop this stuff and remove npmignore

This comment has been minimized.

Copy link
@genediazjr

genediazjr Jun 8, 2016

Author Contributor

No problem. I guess we can just close this pull since gitignore will suffice for this purpose.

@arb

This comment has been minimized.

Copy link
Contributor

arb commented Jun 7, 2016

API.md can stay too; if makes sense if you're offline.

@Marsup

This comment has been minimized.

Copy link
Member

Marsup commented Jun 7, 2016

@arb we didn't really agree on that on contrib.

@arb

This comment has been minimized.

Copy link
Contributor

arb commented Jun 7, 2016

Who are we waiting on? Does this have to be something that is the same for every repo? A bunch of other repos have already added a .npmignore file without waiting for consensus.

@Marsup

This comment has been minimized.

Copy link
Member

Marsup commented Jun 7, 2016

Fair enough.

@arb arb closed this Jun 8, 2016
@arb

This comment has been minimized.

Copy link
Contributor

arb commented Jun 8, 2016

Sorry for all the churn and thanks for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.