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

.cfignore file causing problems with the npm package tmp #45

Closed
pmuellr opened this issue Jul 24, 2020 · 5 comments
Closed

.cfignore file causing problems with the npm package tmp #45

pmuellr opened this issue Jul 24, 2020 · 5 comments
Assignees

Comments

@pmuellr
Copy link
Member

pmuellr commented Jul 24, 2020

I just got a report that the .cfignore file in this repo is causing a problem when trying to use the npm package tmp.

node-cfenv/.cfignore

Lines 1 to 3 in 4103a3e

tmp
lib-src

I suppose that's kinda expected, and unfortunate, but I wonder how a .cfignore file in one package could impact another, unless that package is somehow installed underneath this one - it might be, from one of the other pre-reqs of this package. 🤷

Anyhoo, it's probably a bit overkill to have this .cfignore file in here anyway, as it's used when you push this repo up as a CF app itself, to test it. It can live with some extra stuff ...

@pmuellr
Copy link
Member Author

pmuellr commented Jul 24, 2020

I believe @JenGoldstrich is going to fix this ...

@JenGoldstrich
Copy link
Collaborator

Hey @pmuellr

Yes! Will submit a commit adding an .npmignore shortly to minimize disruption, it'll just prevent the .cfignore from getting included in the node package.

For some context this is definitely undesirable on the cf-cli side but it is going to take us some time to think about how we want to fix how we scan for ignore files.

Thank you!
Jenna

@pmuellr
Copy link
Member Author

pmuellr commented Jul 24, 2020

I'm actually not a fan of .npmignore files, as you then have to duplicate all the things they do by default.

It seems like deleting the .cfignore files will be easier, and the downside is very low (the image used for testing the package by pushing this repo will be a little bit bigger).

@JenGoldstrich
Copy link
Collaborator

Thats definitely a valid point, will delete the .cfignore!

JenGoldstrich pushed a commit that referenced this issue Jul 24, 2020
@JenGoldstrich
Copy link
Collaborator

I have pushed v1.2.3 with this fix and published it to npm, closing this issue, thank you so so much again @pmuellr !

Jenna

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