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

Ignore unnecessary files in the release #334

Merged
merged 3 commits into from
Jul 8, 2023
Merged

Conversation

angeljqv
Copy link
Contributor

@angeljqv angeljqv commented Jul 7, 2023

Closes #332

If you are in a position to help get that fixed for the 3.x branch, a PR is welcome as we are soon to release 3.2.

@cjbarth is it alright???

@cjbarth
Copy link
Contributor

cjbarth commented Jul 8, 2023

That's pretty good, but you have too much there. Please see https://docs.npmjs.com/cli/v9/configuring-npm/package-json#files

@cjbarth cjbarth merged commit 4b1e39e into node-saml:3.x Jul 8, 2023
@cjbarth cjbarth added the chore label Jul 8, 2023
@angeljqv angeljqv deleted the patch-1 branch July 10, 2023 13:54
@parallels999
Copy link
Contributor

Great 👍

package.json Outdated
@@ -24,7 +24,6 @@
"files": [
"CHANGELOG.md",
"index.d.ts",
"index.js",
Copy link
Contributor

@parallels999 parallels999 Jul 11, 2023

Choose a reason for hiding this comment

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

@cjbarth hi, Is this change correct?

I already read this: configuring-npm/package-json#files

Certain files are always included, regardless of settings: The file in the "main" field

But with npm pack i get this:

image

index.js is missing, am i doing something wrong?? or is main failing because it is ./index.js

Copy link
Contributor

Choose a reason for hiding this comment

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

>npm pack

npm notice package: xml-crypto@3.1.0
npm notice === Tarball Contents ===
npm notice 8.5kB  CHANGELOG.md
npm notice 1.1kB  LICENSE
npm notice 19.4kB README.md
npm notice 3.3kB  index.d.ts
npm notice 8.2kB  lib/c14n-canonicalization.js
npm notice 1.4kB  lib/enveloped-signature.js
npm notice 9.7kB  lib/exclusive-canonicalization.js
npm notice 422B   lib/file-key-info.js
npm notice 35.6kB lib/signed-xml.js
npm notice 1.4kB  lib/string-key-info.js
npm notice 2.5kB  lib/utils.js
npm notice 2.0kB  package.json
npm notice === Tarball Details ===
npm notice name:          xml-crypto
npm notice version:       3.1.0
npm notice filename:      xml-crypto-3.1.0.tgz
npm notice package size:  21.5 kB
npm notice unpacked size: 93.5 kB
npm notice shasum:        d1ed360c253661f590fb36f310eee0ee9ef3bf73
npm notice integrity:     sha512-aemV+HLB4L98+[...]7f8k90hq/Fr2g==
npm notice total files:   12
npm notice
xml-crypto-3.1.0.tgz

Copy link
Contributor

@parallels999 parallels999 Jul 11, 2023

Choose a reason for hiding this comment

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

if you remove ./ it works

xml-crypto/package.json

Lines 22 to 23 in 6bcbaa6

"main": "./index.js",
"types": "./index.d.ts",

Maybe here too
"lib": "./lib"

Example: @xmldom/xmldom/package.json#L20-L21

Copy link
Contributor

Choose a reason for hiding this comment

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

Good find. I was just about to do a release. Please make a PR for this and we'll get it in :) I've been jumping around too much and missed this. Thanks for keeping an eye on this!

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.

3 participants