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

internal/report: error if filter argument has no matches #631

Merged
merged 1 commit into from
Jul 15, 2021

Conversation

prattmic
Copy link
Member

list, weblist, disasm, and peek all take a regexp argument that acts as
a filter.

Currently if this filter results in no matches we simply generate an
empty report, which can be confusing to users. Instead, explicitly treat
this as an error condition that is clearly reported to users.

Fixes #629

@google-cla google-cla bot added the cla: yes label May 11, 2021
@prattmic
Copy link
Member Author

I took the same approach with weblist as the rest of the commands, but I'm not totally confident in this. Since weblist has HTML output, it might be cleaner to include the error in the output HTML rather than failing generation entirely?

@codecov-commenter
Copy link

codecov-commenter commented Jun 25, 2021

Codecov Report

Merging #631 (fe317d6) into master (a478d1d) will increase coverage by 0.60%.
The diff coverage is n/a.

❗ Current head fe317d6 differs from pull request most recent head ba2b8a6. Consider uploading reports for the commit ba2b8a6 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #631      +/-   ##
==========================================
+ Coverage   67.29%   67.89%   +0.60%     
==========================================
  Files          40       40              
  Lines        7310     7320      +10     
==========================================
+ Hits         4919     4970      +51     
+ Misses       1952     1909      -43     
- Partials      439      441       +2     
Impacted Files Coverage Δ
.../github.com/google/pprof/internal/report/source.go 80.42% <0.00%> (+0.93%) ⬆️
.../github.com/google/pprof/internal/report/report.go 28.54% <0.00%> (+6.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a478d1d...ba2b8a6. Read the comment docs.

internal/report/report.go Outdated Show resolved Hide resolved
list, weblist, disasm, and peek all take a regexp argument that acts as
a filter.

Currently if this filter results in no matches we simply generate an
empty report, which can be confusing to users. Instead, explicitly treat
this as an error condition that is clearly reported to users.

Fixes google#629
@prattmic
Copy link
Member Author

PTAL

@aalexand aalexand merged commit 86eeefc into google:master Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No warning if filter argument has no matches.
3 participants