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 min and max implementation for Cyclomatic #699

Merged
merged 2 commits into from
Nov 25, 2021

Conversation

giovannitangredi
Copy link
Contributor

@giovannitangredi giovannitangredi commented Nov 18, 2021

This PR adds minimum and maximum value for the Cyclomatic metric, adding some new unit testing to handle some corner cases and partly resolving Issue #410 .

@giovannitangredi giovannitangredi changed the title Add min and max implementation for Cognitive Add min and max implementation for Cyclomatic Nov 18, 2021
@calixteman
Copy link
Collaborator

It would be nice (if it's possible) to add some references (papers, websites, ...) which could help to explain why min/max of cyclomatic is interesting. If they don't exist or if they aren't interesting, it isn't a problem, it's more out of curiosity (and I make some archeology on github I always appreciate to have some documentation (even if I don't do it myself all the time ;))).
Anyway we compute mean, so why not min/max, median, ... whatever I don't know.

@codecov-commenter
Copy link

Codecov Report

Merging #699 (cde25d8) into master (a507c7d) will increase coverage by 0.36%.
The diff coverage is 78.68%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #699      +/-   ##
==========================================
+ Coverage   37.16%   37.52%   +0.36%     
==========================================
  Files          50       50              
  Lines        6294     6348      +54     
  Branches      936      940       +4     
==========================================
+ Hits         2339     2382      +43     
- Misses       3311     3318       +7     
- Partials      644      648       +4     
Impacted Files Coverage Δ
rust-code-analysis-web/src/web/server.rs 60.54% <ø> (ø)
src/metrics/cyclomatic.rs 74.81% <76.78%> (+1.55%) ⬆️
src/spaces.rs 65.69% <100.00%> (+1.02%) ⬆️

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 a507c7d...cde25d8. Read the comment docs.

@giovannitangredi
Copy link
Contributor Author

giovannitangredi commented Nov 23, 2021

I have done the following changes:
Removed self_cyclomatic.

Added a new field called cyclomatic_sum that stores the sum of all cyclomatic for each space merged, similar to the old usage of cyclomatic. Now this value is used as sum and it is also used for calculate average. This field is merged when the merge funciton is called.

Now cyclomatic only stores the cyclomatic value of the single space/method, it doesn't get modified by the merge. This value is used when computing min and max and it is added to the sum when the parsing of the space is completed (implemented in the compute_minmax function of cyclomatic).

This changes have been done because when we merge 2 states, the first one could still be uncompleted and so the min and max cannot be computed correctly, now the min and max are only computed when a state has been completed.

Copy link
Collaborator

@calixteman calixteman left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for doing that.

@calixteman calixteman merged commit a6fb78b into mozilla:master Nov 25, 2021
@Luni-4 Luni-4 deleted the min-max-metrics branch November 25, 2021 16:51
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