Skip to content

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented Oct 1, 2025

Fixes two typo-related bugs:

  • ignoring bundled edges in the isolate reifier when assigning common properties in the tree
  • propagating legacy peer deps in children when calculating place-dep.

jsoref added 10 commits October 1, 2025 08:16
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
@jsoref jsoref requested a review from a team as a code owner October 1, 2025 12:57
@jsoref jsoref force-pushed the spelling-workspaces-arborist branch 2 times, most recently from d4adc19 to 2e19606 Compare October 1, 2025 15:17
@wraithgar wraithgar changed the title chore: fix spelling in arborist fix: spelling Oct 1, 2025
@wraithgar wraithgar changed the title fix: spelling fix: bundled dependency attributes in isolated reifier Oct 1, 2025
@wraithgar wraithgar changed the title fix: bundled dependency attributes in isolated reifier fix: bundled dependency common properties in isolated reifier Oct 1, 2025
@wraithgar wraithgar changed the title fix: bundled dependency common properties in isolated reifier fix: typo bugs and other spelling fixes Oct 1, 2025
@wraithgar
Copy link
Member

First and foremost, this work is already important on its own. Fixing readability in comments and code is important. Thank you.

BUT. In a classic case of curb cutting, you also found two bugs while doing this. Great job. We'll have to work through what tests are failing now and why, but at this point we can assume the tests are wrong since it was a code fix that made them start failing.

@wraithgar
Copy link
Member

Can we revert the changes in the registry mocks? Those are fixtures pulled from the registry itself.

@jsoref jsoref force-pushed the spelling-workspaces-arborist branch from 2e19606 to 7517783 Compare October 1, 2025 16:56
jsoref added 8 commits October 1, 2025 12:57
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
jsoref added 19 commits October 1, 2025 12:57
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
@jsoref jsoref force-pushed the spelling-workspaces-arborist branch from 7517783 to 9b34d43 Compare October 1, 2025 16:57
@jsoref
Copy link
Contributor Author

jsoref commented Oct 1, 2025

Fwiw, I think most of the test failures were me doing a not great job splitting some commits across subtrees and occasionally making last minute changes -- classic ways I mess up.

Thankfully the ci is reasonably friendly. If I could more easily run it in my forks, I wouldn't have included them in the PRs, but almost all of the ci actively suppresses itself in forks. I appreciate the concern it has for fork resources. And yes, if I really really wanted to I could have changed the CI to run in my forks, but I decided that this org wanted me to do PRs publicly, so I left things as is.

My only complaint about the failures is that I didn't immediately know to search for not ok. It'd be nice if failing tests said:

Search for not ok to identify failures.

... once at the very end. As someone who does a lot of drive-by contributing to many disparate ecosystems, this tends to be one of the things I wish everyone did a better job of instead of assuming that all users would know the proper magical search term for their specific CI tool.

@wraithgar
Copy link
Member

My only complaint about the failures is that I didn't immediately know to search for not ok. It'd be nice if failing tests said:

Search for not ok to identify failures.

... once at the very end. As someone who does a lot of drive-by contributing to many disparate ecosystems, this tends to be one of the things I wish everyone did a better job of instead of assuming that all users would know the proper magical search term for their specific CI tool.

This is a great suggestion. Unfortunately it's not something I know how to do easily given the current testing setup. We'd either have to create a custom test script that calls the real tests, and outputs something specific on error. Or we'd have to write a custom reporter for tap. Neither of those seems very elegant or practical.

Those package.json#scripts entries are centrally managed in @npmcli/template-oss so a good place to start would be an issue there for tracking: npm/template-oss#528

@wraithgar wraithgar merged commit 0a8b8c2 into npm:latest Oct 1, 2025
16 checks passed
@jsoref jsoref deleted the spelling-workspaces-arborist branch October 1, 2025 18:06
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.

2 participants