Git Product home page Git Product logo

Comments (6)

tommy-mitchell avatar tommy-mitchell commented on September 27, 2024 1

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.

Drarig29 avatar Drarig29 commented on September 27, 2024 1

@tommy-mitchell yes, I'll do this next week-end

from np.

Drarig29 avatar Drarig29 commented on September 27, 2024

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: 1
  • test/_utils-1.js: 1
  • test/_utils-2.js: 1
  • test/_utils-3.js: 1
  • test/_utils-4.js: 1
  • test/_utils-5.js: 1
  • test/_utils-6.js: 1
  • test/_utils-7.js: 1
  • test/_utils-8.js: 1
  • test/_utils-9.js: 1
  • test/_utils-10.js: 1
  • test/_utils-11.js: 1
  • test/_utils-12.js: 1
  • test/_utils-13.js: 1
  • test/fixtures/config: 6
  • test/fixtures/files: 48
  • test/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.

tommy-mitchell avatar tommy-mitchell commented on September 27, 2024

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.

Drarig29 avatar Drarig29 commented on September 27, 2024

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.

tommy-mitchell avatar tommy-mitchell commented on September 27, 2024

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)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo 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.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.