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

Move skipBadPublishes() -> calculate-versions #450

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jablko
Copy link
Contributor

@jablko jablko commented May 5, 2022

Can we move skipBadPublishes() to the calculate-versions script, instead of calculating the incipient stub version from cached npm info twice, in generate-packages and publish-packages?

skipBadPublishes() calculates the version of the not-needed stub that we're about to publish. It depends on calculate-versions for cached npm-info. For regular packages, calculate-versions is where we actually perform that step (of figuring out the version we're about to publish).

What I did was:

  • Move skipBadPublishes() to calculate-versions.ts. Combine it and isAlreadyDeprecated() into fetchIncipientStubVersion().
  • Add that version to versions.json (the calculate-versions output), alongside the regular packages' versions-to-publish.
  • Use it to replace skipBadPublishes() in generate-packages and publish-packages.

@jablko
Copy link
Contributor Author

jablko commented May 22, 2022

I've rebased this on #451 and added a couple of tests for fetchIncipientStubVersion(), similar to the tests for fetchTypesPackageVersionInfo() I propose adding in #464.

@jablko jablko force-pushed the patch-46 branch 3 times, most recently from c7187a7 to f31dd40 Compare June 1, 2022 20:43
@jablko jablko force-pushed the patch-46 branch 3 times, most recently from c3908b7 to 3bccd9b Compare June 10, 2022 14:31
@sandersn
Copy link
Member

With the new method of deprecation, I think the right thing is to remove skipBadPublishes entirely since it existed to skip the fallout from the publish/deprecate race condition on npm.

@jablko
Copy link
Contributor Author

jablko commented Jun 24, 2022

@sandersn Are there any other ways for a @types package version to get ahead of its not-needed version? The npm-naming rule should ensure we don't publish a minor version that doesn't already exist on npm, but there are some types in the repo from before that rule was a thing, e.g. angular-ui-router.

I can imagine some other ways in theory it could happen (the JS package adds types in a patch version and our patch version is already greater, they add types in a new major/minor but we publish a corresponding @types version before adding it to notNeededPackages.json).

You're right about removing skipBadPublishes() from publish-packages.ts since the new method of deprecation, but I wonder if its worth keeping for other cases?

@jablko jablko force-pushed the patch-46 branch 5 times, most recently from a1c961d to bee9e20 Compare June 28, 2022 19:20
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