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

Add internal/trace from golang #55

Merged
merged 3 commits into from May 13, 2020
Merged

Conversation

povilasv
Copy link
Collaborator

@povilasv povilasv commented May 10, 2020

Currently adding that package to internal/trace, as I feel we shouldn't be exposing others author's packages for them. But if maintainers think it's more reasonable to make it non internal, then I can change the PR :)

The only thing I actually added is README.md, so it makes sense to only review that :)

Ref #46

@povilasv
Copy link
Collaborator Author

It would be awesome to get this reviewed/merged as a lot of other PRs that I would like to open depends on this.

@brancz
Copy link
Member

brancz commented May 12, 2020

Could we add some automation on updating this? Otherwise stuff like this tends to get fragile quickly.

@povilasv
Copy link
Collaborator Author

👍 I'll do something like make sync-trace which git pull go repo and put it into right place

@brancz
Copy link
Member

brancz commented May 12, 2020

sounds good to me

@povilasv
Copy link
Collaborator Author

Done :)

Copy link
Member

@brancz brancz left a comment

Choose a reason for hiding this comment

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

Just one nit regarding review-ability of this long term. Otherwise lgtm!

@@ -0,0 +1,825 @@
// Copyright 2017 The Go Authors. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

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

I think github has a couple of magic headers that can be used to auto hide “generated” content. Do you mind adding one of those so the diff when we sync isn’t as huge to review as it is now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

lovely

@brancz brancz merged commit 57ea870 into parca-dev:master May 13, 2020
@povilasv povilasv deleted the add-trace-lib branch May 13, 2020 10:16
brancz pushed a commit that referenced this pull request Oct 5, 2021
pkg/storage: Move value chunks from tree nodes to a map in the series
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.

None yet

2 participants