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

feat: Changes to node cyclotron package #24481

Merged
merged 25 commits into from
Aug 21, 2024

Conversation

benjackwhite
Copy link
Contributor

@benjackwhite benjackwhite commented Aug 20, 2024

Problem

Had a few gotchas along the way that I thought would be good to get sorted already. Not trying to do too much here

Changes

  • Move the cyclotron types and wrappers into the repo itself with a build step
  • Actually link the package properly via file:
  • Annoyingly file: linking doesn't take care of the build steps for us but this works well enough for now until we later
  • Moved initialisation into the cdp consumer start functions

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Does this work well for both Cloud and self-hosted?

How did you test this code?

@benjackwhite benjackwhite marked this pull request as ready for review August 20, 2024 13:11
Copy link
Contributor

github-actions bot commented Aug 20, 2024

Size Change: 0 B

Total Size: 1.08 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 1.08 MB

compressed-size-action

Copy link
Contributor

@bretthoerner bretthoerner left a comment

Choose a reason for hiding this comment

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

Looks good other than a build issue I've detailed.

rust/cyclotron-node/package.json Show resolved Hide resolved
@benjackwhite benjackwhite merged commit d88b46a into oliver_cyclotron_lib Aug 21, 2024
82 of 83 checks passed
@benjackwhite benjackwhite deleted the feat/ben-cyclotron-changes branch August 21, 2024 11:53
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