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: send projectId and branchId to Segment #309

Merged
merged 2 commits into from
Dec 20, 2024

Conversation

PlayLikeNeverB4
Copy link
Contributor

@PlayLikeNeverB4 PlayLikeNeverB4 commented Dec 19, 2024

Relates to https://github.com/neondatabase/cloud/issues/20941

Because the cli_started event is sent at the beginning we can't know the resource ids.

So I added a cli_command_success event after the command runs when we have the final context.

These will not be sent on error because I couldn't find a way to get the context on error.

Examples

neon branches list will send projectId

neon branches get someid will send projectId and branchId

neon branches get main will send projectId and correct branchId

branches restore A B will send projectId and branchId=A

neon me will send both as undefined

@davidgomes
Copy link
Collaborator

@PlayLikeNeverB4 this PR also seems to include some commits that are not part of this change

@duskpoet
Copy link
Member

That's probably my fault, I had to fix main with force-push :3

@PlayLikeNeverB4
Copy link
Contributor Author

Fixed it

Copy link
Member

@duskpoet duskpoet left a comment

Choose a reason for hiding this comment

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

I am afraid that this approach is too unintuitive and new additions may fail to follow it. What about adding this logic to the helper method branchIdFromProps?

@PlayLikeNeverB4
Copy link
Contributor Author

@duskpoet I implemented your suggestion. Also noticed that I needed to treat restore separately.

src/commands/branches.ts Show resolved Hide resolved
src/index.ts Show resolved Hide resolved
@PlayLikeNeverB4 PlayLikeNeverB4 merged commit f53c5c4 into main Dec 20, 2024
7 checks passed
@PlayLikeNeverB4 PlayLikeNeverB4 deleted the george/track-project-id branch December 20, 2024 10:17
github-merge-queue bot pushed a commit that referenced this pull request Dec 20, 2024
Relates to neondatabase/cloud#20941

Because the `cli_started` event is sent at the beginning we can't know
the resource ids.

So I added a `cli_command_success` event after the command runs when we
have the final context.

These will not be sent on error because I couldn't find a way to get the
context on error.

### **Examples**

`neon branches list` will send `projectId`

`neon branches get someid` will send `projectId` and `branchId`

`neon branches get main` will send `projectId` and correct `branchId`

`branches restore A B` will send `projectId` and `branchId=A`

`neon me` will send both as `undefined`
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.

3 participants