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

don't pack pems #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

don't pack pems #8

wants to merge 1 commit into from

Conversation

calvinmetcalf
Copy link
Contributor

fix for #7

@dcousens
Copy link
Member

NACK. As discussed in #7, this isn't our problem.

@calvinmetcalf
Copy link
Contributor Author

we've troubleshooted this two times already, even though this isn't our problem, I don't want to troubleshoot this a 3rd time or a 7th time.

@dcousens
Copy link
Member

Just leave the thread as a warning for others?

@calvinmetcalf
Copy link
Contributor Author

So eventually they will make it here, but they may open an issue several other places first before they get to here and find this. Yes it is 100% the users fault for accidentally including node_modules but we may be able to prevent those users from wasting a lot of other people's time as they try to figure out what the issue is.

@dcousens
Copy link
Member

It hasn't wasted that much time?
The issue should be quite easily google-able now.

@Ajnasz
Copy link

Ajnasz commented Aug 4, 2017

Is it possible to merge this and publish a package without the tests? I don't pack node_modules, it's just a disturbing warning during extension development in chrome when you load unpacked extension.

And besides of the chrome extension development issue, why do you need to pack tests into the production package?

@yss14
Copy link

yss14 commented Nov 21, 2018

@Ajnasz I totally agree with you. This has nothing to do with accidentally including node_modules into their chrome extension, but is just annoying during development. @dcousens Is there any suitable reason to not ship the test directory? Instead of having a long discussion about including node_modules into chrome extensions, we just could focus on the main problem and solve it...

@dcousens
Copy link
Member

dcousens commented Nov 21, 2018

Just ignore test/, NACK for these files only

@octref octref mentioned this pull request Jun 29, 2019
@zhr1130
Copy link

zhr1130 commented Dec 4, 2019

Can we merge it to master ? It's against our security scan

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