-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Enable top and div args for printing Type::avg maps #1416
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two other things:
- can you add a description of the change in the commit message?
- can you add some tests? runtime tests seems like the right place to put it. It'll help prevent future regressions
2c9c4a2
to
8d3f9d1
Compare
Add runtime tests. |
Seems like the test has an off-by-one issue |
7b3d89c
to
3d693b7
Compare
Fixed the issue and add a runtime test for json format output. Seems like some tests are queued and won't run. Guess I'll push it later to rerun the tests. |
The top and div arguments of `print()` currently does not work for avg maps because it wasn't implemented. It makes sense to have them supported. Resolves bpftrace#1385.
Push again to rerun the tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Can you follow this up with a semantic analyser change to disallow using print and div args with stats()
?
Sure, already done:) #1433. |
The merged PR which enables `top` and `div` args of `print` for `avg` maps has two issues that need addressing: 1. It came with subtractions of unsigned integers which can easily cause negative overflows. 2. The runtime tests it came with are somehow unstable and fails randomly. This patch brings more straightforward tests.
The merged PR which enables `top` and `div` args of `print` for `avg` maps has two issues that need addressing: 1. It came with subtractions of unsigned integers which can easily cause negative overflows. 2. The runtime tests it came with are somehow unstable and fails randomly. This patch brings more straightforward tests.
The merged PR which enables `top` and `div` args of `print` for `avg` maps has two issues that need addressing: 1. It came with subtractions of unsigned integers which can easily cause negative overflows. 2. The runtime tests it came with are somehow unstable and fails randomly. This patch brings more straightforward tests.
The merged PR which enables `top` and `div` args of `print` for `avg` maps has two issues that need addressing: 1. It came with subtractions of unsigned integers which can easily cause negative overflows. 2. The runtime tests it came with are somehow unstable and fails randomly. This patch brings more straightforward tests.
The merged PR which enables `top` and `div` args of `print` for `avg` maps has two issues that need addressing: 1. It came with subtractions of unsigned integers which can easily cause negative overflows. 2. The runtime tests it came with are somehow unstable and fails randomly. This patch brings more straightforward tests.
The top and div arguments of
print()
currently does not work for avg mapsbecause it wasn't implemented. I think it makes sense to have them supported.
Resolves #1385.
Checklist
docs/reference_guide.md
CHANGELOG.md