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 -centile flag #55

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

add -centile flag #55

wants to merge 6 commits into from

Conversation

hangxie
Copy link

@hangxie hangxie commented Jul 5, 2024

Add this flag to show percentile.

Other thought - if gocyclo plans to support more stats features, SortAndFilter would be better split into Sort and Filter.

@hangxie hangxie force-pushed the percentile branch 3 times, most recently from 455d46c to 813e896 Compare July 6, 2024 14:09
@fzipp
Copy link
Owner

fzipp commented Jul 7, 2024

I assume this is meant to address #54 in a more general way, since the median is the 50th percentile, or was this PR an independent initiative?

I agree that Sort and Filter should probably be separate steps/functions, but you don't have to address it in this PR. I can do that later. Sort could also use the simpler slices.Sort (since Go 1.21) nowadays.

@fzipp
Copy link
Owner

fzipp commented Jul 7, 2024

The output probably needs a label, analogous to the "Average: " label for -avg.

The question is if we need a short option as well. I'm not sure how people want to use it: Do they just want to look at the output on the command line or parse it for further processing in a pipeline?

@fzipp fzipp mentioned this pull request Jul 7, 2024
@hangxie
Copy link
Author

hangxie commented Jul 7, 2024

Committed a new change to have label for percentile, also include centile-short to eliminate label, I don't think the change is perfect but it works.

A couple other thoughts triggered but the new commit:

  1. maybe have a -json to output things in JSON format, it can make life easier when people want to parse output
  2. replace all *-short with a global no-label CLI parameter, this will break backward compatibility though and may have a sematic problem, like user asks for avg and 90-th percentile, without label we will have to document their sequence in output.

cmd/gocyclo/main.go Outdated Show resolved Hide resolved
cmd/gocyclo/main.go Outdated Show resolved Hide resolved
@hangxie
Copy link
Author

hangxie commented Jul 7, 2024

Let me know if you want me rebase to squash to single commit.

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