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

(#215) LCOM4 static fields are now called by FQN #307

Merged
merged 8 commits into from
Jan 25, 2019

Conversation

ilyakharlamov
Copy link
Contributor

This PR:

  • fixes LCOM4.xsl:55-57: LCOM4: #156 needs to be fixed in order... #215 (now static fields have FQN - Fully Qualified Name)
  • All the LCOM4 Tests are moved out of MetricsTest.java to a separate Lcom4Test.java
  • Additional descriptions are provided for every Lcom4Test
  • A new MetricBase helper class in the test that helps to do variable assertions much easier.
  • A previously created ignored test findFieldWithQualifiedName is now active (was @Ignore)
  • A new PDD todo

@0crat 0crat added the scope label Jan 23, 2019
@0crat
Copy link
Collaborator

0crat commented Jan 23, 2019

Job #307 is now in scope, role is REV

@codecov-io
Copy link

codecov-io commented Jan 23, 2019

Codecov Report

Merging #307 into master will increase coverage by 0.11%.
The diff coverage is 90%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #307      +/-   ##
============================================
+ Coverage     75.36%   75.47%   +0.11%     
- Complexity      182      183       +1     
============================================
  Files            33       34       +1     
  Lines          1088     1097       +9     
  Branches         88       88              
============================================
+ Hits            820      828       +8     
- Misses          237      238       +1     
  Partials         31       31
Impacted Files Coverage Δ Complexity Δ
...rc/main/java/org/jpeek/skeleton/QualifiedName.java 100% <100%> (ø) 1 <1> (?)
src/main/java/org/jpeek/skeleton/OpsOf.java 92.85% <85.71%> (-2.6%) 6 <0> (ø)

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 4216264...65f399d. Read the comment docs.

@0crat
Copy link
Collaborator

0crat commented Jan 23, 2019

This pull request #307 is assigned to @fabriciofx/z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @paulodamaso/z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewer; there will be no monetary reward for this job

Copy link

@fabriciofx fabriciofx left a comment

Choose a reason for hiding this comment

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

@ilyakharlamov Done. Check out.

src/main/java/org/jpeek/skeleton/OpsOf.java Outdated Show resolved Hide resolved
src/test/java/org/jpeek/metrics/Lcom4Test.java Outdated Show resolved Hide resolved
src/test/java/org/jpeek/metrics/Lcom4Test.java Outdated Show resolved Hide resolved
src/test/java/org/jpeek/metrics/Lcom4Test.java Outdated Show resolved Hide resolved
src/test/java/org/jpeek/metrics/MetricBase.java Outdated Show resolved Hide resolved
src/test/java/org/jpeek/metrics/MetricBase.java Outdated Show resolved Hide resolved
src/test/java/org/jpeek/metrics/MetricBase.java Outdated Show resolved Hide resolved
src/test/java/org/jpeek/metrics/MetricBase.java Outdated Show resolved Hide resolved
src/test/java/org/jpeek/metrics/MetricBase.java Outdated Show resolved Hide resolved
@fabriciofx
Copy link

@ilyakharlamov This PR seems fine to me. Congratulations! Well done!

@fabriciofx
Copy link

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Jan 25, 2019

@rultor merge

@fabriciofx Thanks for your request. @paulodamaso Please confirm this.

Copy link
Collaborator

@paulodamaso paulodamaso left a comment

Choose a reason for hiding this comment

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

@ilyakharlamov Just one comment, please take a look

Copy link
Collaborator

@paulodamaso paulodamaso left a comment

Choose a reason for hiding this comment

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

@ilyakharlamov Thanks for the fixes, there are some questions please take a look

src/main/java/org/jpeek/skeleton/QualifiedName.java Outdated Show resolved Hide resolved
src/test/java/org/jpeek/metrics/Lcom4Test.java Outdated Show resolved Hide resolved
src/test/java/org/jpeek/metrics/Lcom4Test.java Outdated Show resolved Hide resolved
src/test/java/org/jpeek/metrics/Lcom4Test.java Outdated Show resolved Hide resolved
src/test/java/org/jpeek/metrics/Lcom4Test.java Outdated Show resolved Hide resolved
src/test/java/org/jpeek/metrics/MetricBase.java Outdated Show resolved Hide resolved
src/test/java/org/jpeek/metrics/MetricBase.java Outdated Show resolved Hide resolved
new IsCloseTo(
value,
// @checkstyle MagicNumberCheck (1 line)
0.001d
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ilyakharlamov As this class is intended to be generican (and I liked this improvement so I think that we could use it in all of our tests) how about receiving this value as a method parameter? WDYT?

Copy link
Contributor Author

@ilyakharlamov ilyakharlamov Jan 25, 2019

Choose a reason for hiding this comment

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

@paulodamaso Agreed. Although it's a bit annoying to type in the rounding error each time, having it hardcoded value is also not great. On another hand, having the value in the tests also have issues, since checkstyle will complain Checkstyle: '0.001' is a magic number. Implemented.

Copy link
Collaborator

@paulodamaso paulodamaso left a comment

Choose a reason for hiding this comment

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

@ilyakharlamov Perfect, thanks!

Copy link
Collaborator

@paulodamaso paulodamaso left a comment

Choose a reason for hiding this comment

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

@ilyakharlamov Just correct the error shown in travis and we're good to go

@paulodamaso
Copy link
Collaborator

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Jan 25, 2019

@rultor merge

@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here

@rultor
Copy link
Collaborator

rultor commented Jan 25, 2019

@rultor merge

@ilyakharlamov @paulodamaso Oops, I failed. You can see the full log here (spent 18min)

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 1.511 s
[INFO] Finished at: 2019-01-25T16:07:47+00:00
[INFO] Final Memory: 13M/354M
[INFO] ------------------------------------------------------------------------
++ pwd
+ pdd --source=/home/r/repo --verbose --file=/dev/null
Found 8 lines in /home/r/repo/.pdd
My version is 0.20.4
Ruby version is 2.3.3 at x86_64-linux
Reading /home/r/repo
Excluding target/**/*
Excluding papers/**/*
Excluding src/site/resources/**/*
179 file(s) found, 422 excluded
Reading status-reports/2019-01-11...
Reading status-reports/2018-12-28.md...
Reading .0pdd.yml...
Reading .github/ISSUE_TEMPLATE.md...
Reading .github/PULL_REQUEST_TEMPLATE.md...
Reading .travis.yml...
Reading LICENSE.txt...
Reading .pdd...
Reading .rultor.yml...
Reading README.md...
Reading .gitignore...
Reading system.properties...
Reading src/test/resources/org/jpeek/samples/ClassAccessingPublicField.java...
Reading src/test/resources/org/jpeek/samples/MethodsWithDiffParamTypes.java...
Reading src/test/resources/org/jpeek/samples/BridgeMethod.java...
Reading src/test/resources/org/jpeek/samples/Bar.java...
Reading src/test/resources/org/jpeek/samples/WithoutAttributes.java...
Reading src/test/resources/org/jpeek/samples/OneCommonAttribute.java...
Reading src/test/resources/org/jpeek/samples/ClassWithPublicField.java...
Reading src/test/resources/org/jpeek/samples/OnlyOneMethodWithParams.java...
Reading src/test/resources/org/jpeek/samples/ClassWithDifferentMethodVisibilities.java...
Reading src/test/resources/org/jpeek/samples/NotCommonAttributesWithAllArgsConstructor.java...
Reading src/test/resources/org/jpeek/samples/Foo.java...
Reading src/test/resources/org/jpeek/samples/NoMethods.java...
Reading src/test/resources/org/jpeek/samples/OneMethodCreatesLambda.java...
Reading src/test/resources/org/jpeek/samples/TwoCommonMethods.java...
Reading src/test/resources/org/jpeek/samples/MethodMethodCalls.java...
Reading src/test/resources/org/jpeek/samples/OneVoidMethodWithoutParams.java...
Reading src/test/resources/org/jpeek/samples/TwoCommonAttributes.java...
Reading src/test/resources/org/jpeek/samples/NotCommonAttributes.java...
Reading src/test/resources/org/jpeek/samples/IndirectlyRelatedPairs.java...
Reading src/test/resources/org/jpeek/samples/OverloadMethods.java...
Reading src/test/resources/log4j.properties...
Reading src/test/java/org/jpeek/IndexTest.java...
Reading src/test/java/org/jpeek/ReportTest.java...
Reading src/test/java/org/jpeek/ReportWithStatisticsTest.java...
Reading src/test/java/org/jpeek/metrics/LormTest.java...
Reading src/test/java/org/jpeek/metrics/Lcom4Test.java...
\u001b[31mERROR\u001b[0m: src/test/java/org/jpeek/metrics/Lcom4Test.java; puzzle at line #76; Space expected at 77:8; make sure all lines in the puzzle body have a single leading space.
If you can't understand the cause of this issue or you don't know how to fix it, please submit a GitHub issue, we will try to help you: https://github.com/yegor256/pdd/issues. This tool is still in its beta version and we will appreciate your feedback. Here is where you can find more documentation: https://github.com/yegor256/pdd/blob/master/README.md.
Exit code is 1
container 3df9137b293ed60513913b7d035585783a277c5f7d370ff1163b8e5094faedda is dead
Fri Jan 25 17:09:05 CET 2019

@paulodamaso
Copy link
Collaborator

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Jan 25, 2019

@rultor merge

@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit 65f399d into cqfn:master Jan 25, 2019
@rultor
Copy link
Collaborator

rultor commented Jan 25, 2019

@rultor merge

@paulodamaso Done! FYI, the full log is here (took me 14min)

@ilyakharlamov ilyakharlamov deleted the i215 branch January 25, 2019 18:46
@0crat
Copy link
Collaborator

0crat commented Jan 25, 2019

Job was finished in 42 hours, bonus for fast delivery is possible (see §36)

@0crat
Copy link
Collaborator

0crat commented Jan 25, 2019

@cucumber007/z please review this job completed by @fabriciofx/z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed

@0crat 0crat removed the scope label Jan 25, 2019
@0crat
Copy link
Collaborator

0crat commented Jan 25, 2019

The job #307 is now out of scope

@0crat
Copy link
Collaborator

0crat commented Jan 25, 2019

Payment to ARC for a closed pull request, as in §28: +10 point(s) just awarded to @paulodamaso/z

@paulodamaso
Copy link
Collaborator

@0crat quality good

@0crat
Copy link
Collaborator

0crat commented Jun 18, 2019

Quality review completed: +4 point(s) just awarded to @paulodamaso/z

@0crat
Copy link
Collaborator

0crat commented Jun 18, 2019

Order was finished, quality is "good": +25 point(s) just awarded to @fabriciofx/z

gsnoff added a commit to gsnoff/jpeek that referenced this pull request Oct 28, 2019
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.

LCOM4.xsl:55-57: LCOM4: #156 needs to be fixed in order...
6 participants