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 pull the example folder into the module build #220

Merged
merged 7 commits into from
Mar 15, 2021

Conversation

dosullivan557
Copy link
Contributor

No description provided.

@dosullivan557
Copy link
Contributor Author

dosullivan557 commented Jan 30, 2021

Resolves #221

Hi @gcharnock @bergie @locnguyen @cava23 - could you review this pr please.

I install your module in an application, it copies over .pem and .key files that are in your tests. This means that when we put our application through security scans, it fails because it finds these files and classifies them as sensitive data. What I'm asking is whether you can not include these in the bundled version of the application.

To resolve it, you can have a section in the package.json that doesn't pull the test files over to the build of the module when it is pulled in.

@drewtunes
Copy link
Contributor

I thought the package-lock.json should never be hand-modded. Is there a corresponding change to package.json you're trying to make? I don't understand the intended change based on the diff.

@dosullivan557
Copy link
Contributor Author

dosullivan557 commented Feb 1, 2021

The package lock has only been upversioned and it wasn't done manually, I was done using the npm version patch command.

@dosullivan557
Copy link
Contributor Author

@cava23 if you look at the latest changes, there is a files section in the package.json that specifies which folders and files need putting into the package when installed.

@drewtunes
Copy link
Contributor

Can you remove whitespace changes so the diff is legible?

@dosullivan557
Copy link
Contributor Author

@cava23 Sure thing - removed whitespace

@drewtunes
Copy link
Contributor

Sorry, looked at this a little closer and it appears this is already being attempted in the .npmignore file. However, I believe the trailing slash is missing. I think the right change here is to update .npmignore to have test/. See if that solves your problem. I'd rather use that than add the files prop to package.json since that could have a greater impact than intended.

@drewtunes
Copy link
Contributor

@dosullivan557
Copy link
Contributor Author

Moved to .gitignore, and added example in there, which is causing an issue too. @cava23

@drewtunes
Copy link
Contributor

Nice, I assume you meant .npmignore instead .gitignore. And this is now working as expected?

@dosullivan557
Copy link
Contributor Author

Yeah sorry I mean .npmignore.

And yep all working as expected

Copy link
Contributor

@drewtunes drewtunes left a comment

Choose a reason for hiding this comment

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

I would like to get approval from one other contributor as I've only made a small change to this project.

@dosullivan557
Copy link
Contributor Author

Sounds fair enough - cheers!

@dosullivan557
Copy link
Contributor Author

Hey @yaronn - are you able to review this pull request, please.

@dosullivan557
Copy link
Contributor Author

Can this please be reviewed? It's causing issues and need this change ASAP.

@cava23
@bergie
@locnguyen
@gcharnock

Copy link
Collaborator

@LoneRifle LoneRifle left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants