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

Use files in package.json #57

Closed
wants to merge 1 commit into from
Closed

Use files in package.json #57

wants to merge 1 commit into from

Conversation

SimenB
Copy link

@SimenB SimenB commented Apr 16, 2015

This avoids installing tests and dotfiles when installing your package

@mafintosh
Copy link
Owner

couldn't this just be *.js (like tar-stream does, https://github.com/mafintosh/tar-stream/blob/master/package.json#L47-L50) to make it easier to maintain?

@SimenB
Copy link
Author

SimenB commented Apr 16, 2015

I think being explicit is easier to maintain. If you add a new file that is needed by consumers, you add it to the array. If it's just a JS-file (maybe you decide to use Grunt/Gulp, for instance), then it's not automatically included.

As of now, this does not include example.js, which is a good example, IMO 😄

@mafintosh
Copy link
Owner

we tried doing that in tar-stream and i kept forgetting to add files to pkg json. tests would still work locally so was hard to catch.

@SimenB
Copy link
Author

SimenB commented Apr 17, 2015

What about moving the js files needed by the package (except for index.js) into a lib directory?

@mafintosh
Copy link
Owner

can you whitelist the entire lib folder then? :)

@SimenB
Copy link
Author

SimenB commented Apr 17, 2015

Yup

On Fri, 17 Apr 2015 07:59 Mathias Buus [email protected] wrote:

can you whitelist the entire lib folder then? :)


Reply to this email directly or view it on GitHub
#57 (comment)
.

@mafintosh
Copy link
Owner

i'd be fine with that 👍

Also Use `files` in package.json
@SimenB
Copy link
Author

SimenB commented Apr 17, 2015

@mafintosh Done 😄

@TrySound
Copy link

@mafintosh Make new release please.

@SimenB
Copy link
Author

SimenB commented Aug 16, 2015

@mafintosh ping

@lucleray
Copy link

@LinusU I'd love to see this PR merged.

capture d ecran 2019-01-21 a 12 12 21

@LinusU
Copy link
Collaborator

LinusU commented Feb 13, 2019

Hey, sorry for the late turnaround on this. I'm definitely 👍 on this.

I don't think that we should move around the files without a major break though. I've seen require('is-my-json-valid/formats') out in the wild, same with /require.

That being said, I don't think that we'll add any new files in the foreseeable future, so I would be okay with listing index.js, formats.js, require.js and files/ in the list, what do you think about that?

Thanks!

edit: oh, and also index.d.ts 🤔

hmm, I wonder if anyone is using /require, maybe it's time to drop that feature 🤔

and if we also want to have, /formats as an entrypoint, we should add a formats.d.ts file as well...

@SimenB
Copy link
Author

SimenB commented Feb 13, 2019

I haven't used this module in years, feel free to tweak this PR or just start from scratch. It seems I've even deleted my fork at some point 😅

@LinusU
Copy link
Collaborator

LinusU commented Feb 13, 2019

Sounds good, thanks for the PR and initiative!

@LinusU LinusU mentioned this pull request Feb 13, 2019
@LinusU LinusU closed this in #175 May 7, 2019
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.

5 participants