Skip to content

Conversation

@fabian-s
Copy link
Contributor

adds (function-wise) linting for cyclomatic complexity.
computes cyclomatic complexity for each entry in source_files$expressions.

incurs a dependency on cyclocomp.
closes #361.

Not sure if the generated message is too verbose for your taste, I figured it's ok since this will only be generated at most once for any top-level expression in each file.

@fabian-s fabian-s requested a review from jimhester as a code owner August 20, 2019 09:11
@codecov
Copy link

codecov bot commented Aug 20, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@2b1bccd). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #396   +/-   ##
=========================================
  Coverage          ?   82.37%           
=========================================
  Files             ?       42           
  Lines             ?     1889           
  Branches          ?        0           
=========================================
  Hits              ?     1556           
  Misses            ?      333           
  Partials          ?        0
Impacted Files Coverage Δ
R/zzz.R 0% <ø> (ø)
R/cyclocomp_linter.R 100% <100%> (ø)

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 2b1bccd...f636ba3. Read the comment docs.

line_number = source_file[["line"]][1],
column_number = source_file[["column"]][1],
type = "style",
message = paste0(
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 this is too long, maybe

"functions should have cyclomatic complexity of less than", complexity_limit, "."

@jimhester
Copy link
Member

jimhester commented Aug 20, 2019

I think this looks good, thanks! Can you please add a bullet to NEWS? It should briefly describe the change and end with (@yourname, #issuenumber).

@jimhester
Copy link
Member

It looks like you have a few test failures, once those are fixed and we have a clean build this can be merged.

@fabian-s
Copy link
Contributor Author

It looks like you have a few test failures, once those are fixed and we have a clean build this can be merged.

sorry about that, forgot to update the tests to the new message... 🙈

@jimhester jimhester merged commit 63d1cce into r-lib:master Aug 23, 2019
@jimhester
Copy link
Member

Great work, this package is now better thanks to you!

Thanks!! You are da bomb!

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.

Linter for function length

2 participants