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

chore(publish): Publish NPM packages in order of dependency tree #8579

Merged
merged 3 commits into from
Jul 19, 2023

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Jul 18, 2023

Following up on #inc-456, this PR splits the previously universal npm target that took care of publishing all packages at once into multiple targets (one for each package).
This will ensure that packages are published in the correct order of their sentry dependency tree (e.g. core -> browser -> react, etc..) Furthermore, this theoretically also lets us disable the release of individual packages (which we might want to do if a package was already released).

In addition, this PR now moves NPM targets before other targets, so that no GH releases are created if the actual publishing of artifacts failed previously.

The drawback of this solution is that we have to manually add a new target for new packages we added. Therefore, this PR also adjusts our new package release checklist to do this before cutting the first release. Given that we had failing releases in the past because of incorrectly configured new packages, this might actually be beneficial.

With this change, craft is configured to process the following targets in the given order:

[
    "npm[@sentry/types]",
    "npm[@sentry/utils]",
    "npm[@sentry/core]",
    "npm[@sentry-internal/tracing]",
    "npm[@sentry/replay]",
    "npm[@sentry/browser]",
    "npm[@sentry/node]",
    "npm[@sentry/angular-ivy]",
    "npm[@sentry/angular]",
    "npm[@sentry/ember]",
    "npm[@sentry/react]",
    "npm[@sentry/svelte]",
    "npm[@sentry/vue]",
    "npm[@sentry/wasm]",
    "npm[@sentry/integrations]",
    "npm[@sentry/serverless]",
    "npm[@sentry/opentelemetry-node]",
    "npm[@sentry/nextjs]",
    "npm[@sentry/remix]",
    "npm[@sentry/sveltekit]",
    "npm[@sentry/gatsby]",
    "npm[@sentry-internal/typescript]",
    "npm[@sentry-internal/eslint-plugin-sdk]",
    "npm[@sentry-internal/eslint-config-sdk]",
    "npm[@sentry/hub]",
    "npm[@sentry/tracing]",
    "aws-lambda-layer",
    "gcs[browser-cdn-bundles]",
    "github",
    "registry"
]

We might want to further group targets to packages (and split up registry targets while at it) but I think for now, we try this publishing approach and avoid creating another ~25 additional targets.

I tested this new config locally with a local craft installation and --dry-run and it seems to pick up the correct packages for each target (and it'll error if a package was not found).

@Lms24 Lms24 changed the title chore(publish): Split npm craft target into one target per package chore(publish): Publish NPM packages in order of dependency tree Jul 18, 2023
@Lms24 Lms24 force-pushed the lms/chore-craft-npm-release-targets branch from 6582b93 to d130d7b Compare July 18, 2023 12:03
@Lms24 Lms24 marked this pull request as ready for review July 18, 2023 14:51
- name: gcs
id: "browser-cdn-bundles"
Copy link
Member Author

Choose a reason for hiding this comment

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

Decided to also give this an id, as it's not apparent that this target uploads the CDN bundles

@Lms24 Lms24 requested a review from mydea July 18, 2023 14:52
Copy link
Member

@mydea mydea left a comment

Choose a reason for hiding this comment

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

Amazing! Great work, this will hopefully help prevent problems in the future 🚀

@Lms24 Lms24 merged commit f2bdd1f into develop Jul 19, 2023
@Lms24 Lms24 deleted the lms/chore-craft-npm-release-targets branch July 19, 2023 08:45
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