Comments (6)
Should it also check the "glob" made by this grouping doesn’t match a file that will be published? E.g. with a naive implementation:
\_ docs/
\_ _helpers/ (10 files)
\_ some-script.js
\_ .someconfig
\_ actual-doc.md
Unpublished:
- docs/* (13 files)
New:
- docs/actual-doc.md
I’m not sure if it matters, since the latter list will list the file explicitly anyway. I’m just worried that someone might get confused that a unpublished
group includes a file that was/will be published. Maybe the wording should be changed to better reflect what’s being shown? Or at least the published
list should be shown first.
The following new files will be published for the first time:
The following new files and patterns will not be published with your package:
from np.
@tommy-mitchell yes, I'll do this next week-end
from np.
I found an implementation which turns this:
- test/_utils.js
- test/_utils-1.js
- test/_utils-2.js
- test/_utils-3.js
- test/_utils-4.js
- test/_utils-5.js
- test/_utils-6.js
- test/_utils-7.js
- test/_utils-8.js
- test/_utils-9.js
- test/_utils-10.js
- test/_utils-11.js
- test/_utils-12.js
- test/_utils-13.js
- test/fixtures/config/homedir4/.np-config.js
- test/fixtures/config/homedir5/.np-config.mjs
- test/fixtures/config/local4/.np-config.js
- test/fixtures/config/local4/package.json
- test/fixtures/config/local5/.np-config.mjs
- test/fixtures/config/package.json
- test/fixtures/files/dot-github/.github/pull_request_template.md
- test/fixtures/files/dot-github/index.js
- test/fixtures/files/dot-github/package.json
- test/fixtures/files/files-and-npmignore/package.json
- test/fixtures/files/files-and-npmignore/readme.md
- test/fixtures/files/files-and-npmignore/source/.npmignore
- test/fixtures/files/files-and-npmignore/source/bar.js
- test/fixtures/files/files-and-npmignore/source/foo.js
- test/fixtures/files/files-and-npmignore/source/index.d.ts
- test/fixtures/files/files-and-npmignore/source/index.test-d.ts
- test/fixtures/files/files-slash/index.js
- test/fixtures/files/files-slash/package.json
- test/fixtures/files/gitignore/dist/index.js
- test/fixtures/files/gitignore/gitignore
- test/fixtures/files/gitignore/index.d.ts
- test/fixtures/files/gitignore/index.js
- test/fixtures/files/gitignore/index.test-d.ts
- test/fixtures/files/gitignore/package.json
- test/fixtures/files/gitignore/readme.md
- test/fixtures/files/has-readme-and-license/index.js
- test/fixtures/files/has-readme-and-license/license.md
- test/fixtures/files/has-readme-and-license/package.json
- test/fixtures/files/has-readme-and-license/readme.md
- test/fixtures/files/main/bar.js
- test/fixtures/files/main/foo.js
- test/fixtures/files/main/package.json
- test/fixtures/files/npmignore-and-gitignore/.npmignore
- test/fixtures/files/npmignore-and-gitignore/dist/index.js
- test/fixtures/files/npmignore-and-gitignore/gitignore
- test/fixtures/files/npmignore-and-gitignore/package.json
- test/fixtures/files/npmignore-and-gitignore/readme.md
- test/fixtures/files/npmignore-and-gitignore/script/build.js
- test/fixtures/files/npmignore-and-gitignore/source/index.ts
- test/fixtures/files/npmignore/.npmignore
- test/fixtures/files/npmignore/index.d.ts
- test/fixtures/files/npmignore/index.js
- test/fixtures/files/npmignore/index.test-d.ts
- test/fixtures/files/npmignore/package.json
- test/fixtures/files/npmignore/readme.md
- test/fixtures/files/one-file/index.js
- test/fixtures/files/one-file/package.json
- test/fixtures/files/source-and-dist-dir/dist/index.js
- test/fixtures/files/source-and-dist-dir/package.json
- test/fixtures/files/source-and-dist-dir/source/bar.js
- test/fixtures/files/source-and-dist-dir/source/foo.js
- test/fixtures/files/source-dir/package.json
- test/fixtures/files/source-dir/source/bar.js
- test/fixtures/files/source-dir/source/foo.js
- test/new-files.js
Into this:
- test/_utils.js
- test/_utils-1.js
- test/_utils-2.js
- test/_utils-3.js
- test/_utils-4.js
- test/_utils-5.js
- test/_utils-6.js
- test/_utils-7.js
- test/_utils-8.js
- test/_utils-9.js
- test/_utils-10.js
- test/_utils-11.js
- test/_utils-12.js
- test/_utils-13.js
- test/fixtures/config/homedir4/.np-config.js
- test/fixtures/config/homedir5/.np-config.mjs
- test/fixtures/config/local4/.np-config.js
- test/fixtures/config/local4/package.json
- test/fixtures/config/local5/.np-config.mjs
- test/fixtures/config/package.json
- test/fixtures/files/* (48 files)
- test/new-files.js
The heuristic is that we create groups with a maximum depth of 3
. Here the groups are:
test/_utils.js
: 1test/_utils-1.js
: 1test/_utils-2.js
: 1test/_utils-3.js
: 1test/_utils-4.js
: 1test/_utils-5.js
: 1test/_utils-6.js
: 1test/_utils-7.js
: 1test/_utils-8.js
: 1test/_utils-9.js
: 1test/_utils-10.js
: 1test/_utils-11.js
: 1test/_utils-12.js
: 1test/_utils-13.js
: 1test/fixtures/config
: 6test/fixtures/files
: 48test/new-files.js
: 1
So yes, it's not folders, but "groups". A file can be a "group" in this implementation.
I think that files near the root of the project are more important to have an idea of the shape of the project, compared to files that are deeper than 3 levels.
And we would collapse a group when its number of files is greater than 10
.
Of course, those magic numbers (maximum depth of 3
and threshold of 10
) are open to discussion.
from np.
The heuristic is that we create groups with a maximum depth of
3
. Here the groups are:
I think that a depth of 2
would be better. Many projects package and exclude whole folders. If I’m releasing a new package, I want to make sure that the test
folder is ignored - I don’t care about its structure. For example:
New:
- package.json
- readme.md
- license.md
- src/* (10 files)
- index.d.ts
Unpublished:
- scripts/* (3 files)
- test/* (12 files)
- index.test-d.ts
- .gitignore
- eslint.config.js
I think grouping or not grouping published
files could go either way.
from np.
Maybe the wording should be changed to better reflect what’s being shown? Or at least the published list should be shown first.
Exactly, the order and the wording should be enough IMO.
I think that a depth of
2
would be better. [...] I want to make sure that the test folder is ignored - I don’t care about its structure.
Good point. To be precise, what you described in this example is a depth of 1
in my implementation. By depth, I mean "the maximum depth of a folder that can be grouped".
I think grouping or not grouping published files could go either way.
I'm just afraid that by grouping published files, we would make the feature of "showing new files that will be published for the first time" useless.
Example: What if among the 10 files in src/*
, I have a src/config/my-config.json.bak
that slipped in with credentials inside it? (because let's say, we only have src/config/*.json
in the .gitignore
)
In this case, either I would notice the "10 files" and would really want to know what's going to be published → in this case I'd need to check myself, so we lost a cool feature of np
because we are lacking the exhaustive list.
Or I don't notice it and as a result, I may have I leaked credentials.
That's why I wanted to provide a way to disable grouping in some way, and the --dry-run
option was my only idea. However, I can't choose whether --dry-run
should or should not do the grouping... 🤔
from np.
Looking back on this, I think the "new published" files should be exhaustively listed. For most cases it shouldn't be too verbose, and if it is we can find ways to mitigate it in the future. Related, I think we can decide on --dry-run
semantics after we get an implementation.
Would you want to make a draft PR with your implementation?
from np.
Related Issues (20)
- np v8 breaking bug: NPM version check fails when `npm` is not specified in engines field in package.json HOT 2
- Won't work if dependencies are not defined
- unable to publish package using Yarn -- cannot read properties of undefined (reading 'pipe') HOT 6
- np command not execute HOT 3
- SyntaxError: Unexpected token '&&=' HOT 1
- How to publish with --provenance?
- Proposal: backwards-compatible compression before publish HOT 2
- Unable to publish preview (`publish.getPackagePublishArguments` is not a function) HOT 1
- Allow creating a GitHub draft release without generating release notes
- ls-collaborators is not a valid access command with npm v9 HOT 5
- np output command error
- np error ERR_REQUIRE_ESM HOT 2
- Provide a way to silently approve in advanced files that won't be part of the published package
- `ENOWORKSPACES` when publishing from workspace HOT 1
- np command extremely slow HOT 7
- how to hook in to publish a bundle with injected version HOT 1
- Doesn't include `dist` folder HOT 5
- np hangs while publishing HOT 4
- Open npmjs.com instead of asking for OTP HOT 1
- after published, print the npm url HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from np.