Skip to content

Cognitive complexity metric for Java - #850

Merged
Luni-4 merged 23 commits into
mozilla:masterfrom
dburriss:dburriss/feature/issue-359/java-cognitive
Jun 28, 2023
Merged

Luni-4 merged 23 commits into
mozilla:masterfrom
dburriss:dburriss/feature/issue-359/java-cognitive

Conversation

@dburriss

@dburriss dburriss commented May 30, 2022 •

Copy link
Copy Markdown
Contributor

Last metric for #359

  • Implementation
  • All tests pass
  • Branch up-to-date
  • Cleanup

Tests and API have changed a fair bit since I started work on this so tracking some additional tasks:

  • update API usage to accommodate changes
  • redo tests with Insta
  • figure out why average not working (something to do with space functions I think)

@dburriss
dburriss force-pushed the dburriss/feature/issue-359/java-cognitive branch from e4845c0 to ca0cf88 Compare January 13, 2023 13:32
@marco-c

marco-c commented Apr 5, 2023

Copy link
Copy Markdown
Collaborator

@dburriss any plans to finish this?

@dburriss

Copy link
Copy Markdown
Contributor Author

Hi @marco-c
Sorry was job hunting and then ramping up so didn't have much energy for this. I'll try dive into this again tomorrow morning now that things have settled.

@dburriss
dburriss force-pushed the dburriss/feature/issue-359/java-cognitive branch from 49ce25c to 1b3821c Compare June 2, 2023 05:28
@Luni-4

Luni-4 commented Jun 15, 2023 •

Copy link
Copy Markdown
Collaborator

Hello @dburriss, thank you for your hard work!

I think that if you insert test methods inside a class, you would get the correct average value. Something like this:

class X {
     public static void print(boolean a, boolean b, boolean c, boolean d){  // +1
         if (a && !(b && c)) { // +3 (+1 &&, +1 &&)
             printf(\"test\");
         }
     }
}

@dburriss

Copy link
Copy Markdown
Contributor Author

Hi @Luni-4
Thanks for the suggestion. I'll give that a try.

@Luni-4

Luni-4 commented Jun 16, 2023

Copy link
Copy Markdown
Collaborator

Hi @Luni-4 Thanks for the suggestion. I'll give that a try.

It worked!

@dburriss
dburriss force-pushed the dburriss/feature/issue-359/java-cognitive branch from 438bc74 to 9ef21cf Compare June 24, 2023 07:03
@dburriss
dburriss marked this pull request as ready for review June 24, 2023 07:44
@dburriss

Copy link
Copy Markdown
Contributor Author

@marco-c @Luni-4 It looks like this is finally ready for a second pair of eyes.

@Luni-4 Luni-4 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you @dburriss! Just simple things from my side:

  • we need to remove the Cargo.lock file since it is no longer necessary
  • we need to replace Boolean with boolean in Java functions

Comment thread src/metrics/cognitive.rs Outdated
Comment thread src/metrics/cognitive.rs Outdated
Comment thread src/metrics/cognitive.rs Outdated
Comment thread src/metrics/cognitive.rs Outdated
@dburriss

Copy link
Copy Markdown
Contributor Author

@Luni-4 thanks for the review. I have made the requested improvements.

@Luni-4 Luni-4 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you @dburriss! Just simple things again

Comment thread src/metrics/cognitive.rs Outdated
Comment thread src/metrics/cognitive.rs Outdated
Comment thread src/metrics/cognitive.rs Outdated
Comment thread src/metrics/cognitive.rs
Comment thread src/metrics/cognitive.rs Outdated

@Luni-4 Luni-4 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ok, last change and we are ready to merge in!

Comment thread src/metrics/cognitive.rs Outdated
@dburriss

dburriss commented Jun 27, 2023 •

Copy link
Copy Markdown
Contributor Author

@marco-c the task expired before a worker was made available for the checks. Should I push another commit to trigger or is there an issue (and trying again could make it worse).

@marco-c

marco-c commented Jun 27, 2023

Copy link
Copy Markdown
Collaborator

I retriggered it manually and it passed.

Luni-4
Luni-4 previously approved these changes Jun 27, 2023

@Luni-4 Luni-4 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It's fine for me! Thanks a lot @dburriss!

@dburriss

Copy link
Copy Markdown
Contributor Author

@marco-c the steps that had not run passed but the rest were in an exception state due to timeout exceeded. Triggered a check with a commit.
@Luni-4 I updated a comment to get the check to run. You may want to check it since I've got those wrong before :p

@dburriss
dburriss requested a review from Luni-4 June 28, 2023 04:59

@Luni-4 Luni-4 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@dburriss Your last commit is fine as well for me! Thank you!

@Luni-4
Luni-4 merged commit 88d957e into mozilla:master Jun 28, 2023
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