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

Refactoring the files to be in correctly nested packages #6120

Merged
merged 7 commits into from
Jan 11, 2025

Conversation

varada610
Copy link
Contributor

@varada610 varada610 commented Jan 8, 2025

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized it.
  • All filenames are in PascalCase.
  • All functions and variable names follow Java naming conventions.
  • All new algorithms have a URL in their comments that points to Wikipedia or other similar explanations.
  • All new code is formatted with clang-format -i --style=file path/to/your/file.java

@codecov-commenter
Copy link

codecov-commenter commented Jan 8, 2025

Codecov Report

Attention: Patch coverage is 86.36364% with 3 lines in your changes missing coverage. Please review.

Project coverage is 73.79%. Comparing base (a9633c0) to head (7646991).

Files with missing lines Patch % Lines
...orithms/matrix/matrixexponentiation/Fibonacci.java 0.00% 2 Missing ⚠️
...healgorithms/matrix/PrintAMatrixInSpiralOrder.java 95.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6120      +/-   ##
============================================
+ Coverage     73.78%   73.79%   +0.01%     
  Complexity     5119     5119              
============================================
  Files           658      658              
  Lines         17632    17632              
  Branches       3391     3391              
============================================
+ Hits          13010    13012       +2     
+ Misses         4118     4117       -1     
+ Partials        504      503       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

siriak
siriak previously approved these changes Jan 10, 2025
Copy link
Member

@siriak siriak left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@siriak
Copy link
Member

siriak commented Jan 10, 2025

Please check why build and linter fail, you probably need to fix formatting to fix them

@siriak
Copy link
Member

siriak commented Jan 10, 2025

Or try updating with the base branch

@varada610
Copy link
Contributor Author

Please check why build and linter fail, you probably need to fix formatting to fix them

Im not sure what fails in the build. locally is fine. It says some PMD check failed not sure how to resolve

@siriak
Copy link
Member

siriak commented Jan 10, 2025

Update your branch first, it may be already resolved in main
image

@vil02
Copy link
Member

vil02 commented Jan 10, 2025

Please check why build and linter fail, you probably need to fix formatting to fix them

Im not sure what fails in the build. locally is fine. It says some PMD check failed not sure how to resolve

Regarding PMD (cf. logs):

Warning:  PMD Failure: com.thealgorithms.matrix.matrixexponentiation.Fibonacci:58 Rule:UnnecessaryFullyQualifiedName Priority:4 Unnecessary qualifier 'Fibonacci': 'IDENTITY_MATRIX' is already in scope because it is declared in an enclosing type.
Warning:  PMD Failure: com.thealgorithms.matrix.matrixexponentiation.Fibonacci:65 Rule:UnnecessaryFullyQualifiedName Priority:4 Unnecessary qualifier 'Fibonacci': 'FIB_MATRIX' is already in scope because it is declared in an enclosing type.

So it should be enough to remove the Fibonacci from the lines 58 and 65.

Regarding clang-format (cf. logs):

--- ./src/test/java/com/thealgorithms/matrix/InverseOfMatrixTest.java	(original)
+++ ./src/test/java/com/thealgorithms/matrix/InverseOfMatrixTest.java	(reformatted)
@@ -1,8 +1,8 @@
 package com.thealgorithms.matrix;
 
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+
 import java.util.stream.Stream;
-
-import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;

i.e. move the static import to the very top.

@vil02
Copy link
Member

vil02 commented Jan 10, 2025

@varada610: as an addition to:
#6120 (comment)

After you will fix the PMD issues, please remove this line:

com.thealgorithms.matrixexponentiation.Fibonacci=UnnecessaryFullyQualifiedName

@varada610
Copy link
Contributor Author

thanks @vil02 and @siriak for the help . i see build is green

@varada610
Copy link
Contributor Author

@siriak @vil02 Require a review to complete the merge

Copy link
Member

@siriak siriak left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@siriak siriak merged commit 1e6ed97 into TheAlgorithms:master Jan 11, 2025
6 checks passed
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.

4 participants